Implement unsafe trait support (continued) #155
Conversation
| #[term] | ||
| #[derive(Default)] | ||
| pub enum Safety { | ||
| #[default] |
There was a problem hiding this comment.
I didn't know you could do this, nifty.
| #[derive(Default)] | ||
| pub enum Safety { | ||
| #[default] | ||
| Safe, | ||
| Unsafe, | ||
| } | ||
|
|
||
| impl fmt::Debug for Safety { |
There was a problem hiding this comment.
Isn't this the default behavior that #[term] would generate?
There was a problem hiding this comment.
Oh I didn't know this, or test that it would. I'll try, and open a PR to clean this up.
crates/formality-check/src/impls.rs
Outdated
| @@ -17,10 +17,8 @@ use formality_types::{ | |||
| }; | |||
|
|
|||
| impl super::Check<'_> { | |||
| #[context("check_trait_impl({v:?})")] | |||
| pub(super) fn check_trait_impl(&self, v: &TraitImpl) -> Fallible<()> { | |||
| let TraitImpl { binder, safety } = v; | |||
There was a problem hiding this comment.
JFYI I use struct patterns a lot as a deliberate style choice -- I want compilation errors when/if add'l fields are added to the struct -- but in this case it doesn't seem very critical.
There was a problem hiding this comment.
It's more for the result when opening binder
There was a problem hiding this comment.
Make sense. I would add this back in this PR but it's already merged :) I'll open another PR.
There was a problem hiding this comment.
Even if it's not critical per se, I've re-introduced it in #156 here for consistency and easier evolvability, as well as to the negative impl counterpart.
| @@ -74,8 +72,8 @@ impl super::Check<'_> { | |||
| } | |||
|
|
|||
| /// Validate that the declared safety of an impl matches the one from the trait declaration. | |||
| fn check_safety_matches(&self, trait_decl: &Trait, trait_impl: &Safety) -> Fallible<()> { | |||
| if trait_decl.safety != *trait_impl { | |||
| fn check_safety_matches(&self, trait_decl: &Trait, trait_impl: &TraitImpl) -> Fallible<()> { | |||
There was a problem hiding this comment.
...and I like this version of the fn better
This PR supersedes #128:
rustfmtas a required component in the toolchain, just so it's easier to format things. And with that, removes some formatting weirdness that breakrustfmt, and formats stray rogue files. I can remove these commits from the PR if you prefer not to have them.