GregoryConrad / rearch-rs

Re-imagined approach to application design and architecture
https://crates.io/crates/rearch
MIT License
84 stars 4 forks source link

CapsuleKey should eagerly impl Hash, Eq, PartialEq, and Debug #31

Closed GregoryConrad closed 10 months ago

GregoryConrad commented 10 months ago

I thought this was a one-line fix with #derive but apparently not. I'm running into an issue with this now:

error[E0119]: conflicting implementations of trait `From<CapsuleKey>` for type `CapsuleKey`
  --> rearch/src/capsule_key.rs:19:1
   |
19 | impl<T: Hash + Eq + Debug + Send + Sync + 'static> From<T> for CapsuleKey {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: conflicting implementation in crate `core`:
           - impl<T> From<T> for T;

That's a little annoying. I can remove any one of the derived traits and then it compiles fine, but that really feels like a hack. I want to keep that From impl since it is really handy, so I am wondering if I should just drop the Hash impl so it all still compiles fine. Looked some more into this, and it is an issue of specialization, which has been an open issue for 8 years or so with no significant progress that I could see. So, I either need to:

Not sure on the best path forward here.

GregoryConrad commented 10 months ago

I think the best path forward is to drop the From impl and instead expose two methods:

This makes the API less ergonomic, but it'll at least be clear what is happening. Maybe the From impl can come back in the future if specialization ever lands.

GregoryConrad commented 10 months ago

This is even more annoying. static is a keyword so it can't be used as a function name. I also don't want to make the enum public because:

All of these options suck.

For now, I think the best course of action is to just exclude the Hash bound and call it a day. When/if specialization ever becomes a thing, we can add the Hash bound then.