-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
prevent specialized negative impls #74648
Conversation
Does this completely forbid "ad-hoc" specialization that was possible before? #![feature(negative_impls)]
#![feature(optin_builtin_traits)]
pub trait Trait {}
pub struct Special;
impl<T> Trait for T where private::Is<T>: private::NotSpecial {}
impl Trait for Special {}
mod private {
pub auto trait NotSpecial {}
pub struct Is<T>(T);
impl !NotSpecial for Is<super::Special> {}
} (playground) |
It does, see the discussion in #74525 (comment) for more details. There are subtle problems rn where negative impls don't work as expected. If you really need this kind of specialization, I would recommend you to use the approach taken in #74254 which shouldn't have the problems caused by partial negative impls. That one does depend on |
@lcnr sadly, my trait actually has an associated type so current implementation of specialization won't work for me :( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks about right, I didn't read it 100% yet, but some of the errors seemed a bit surprising..?
@@ -1,4 +1,4 @@ | |||
error: auto traits must not contain where bounds | |||
error: negative impls are not allowed for tuples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we disallow negative impls for all tuples? (I think that'd be ok, actually, though I think we could also permit them for tuples if they are only restricted based on arity, but I don't see the point.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think allowing them for tuples will prevent us from ever adding variadic tuples, so it should be easier to just disallow them for now
@@ -6,6 +6,7 @@ trait MyTrait { | |||
type Foo; | |||
} | |||
|
|||
default impl !MyTrait for u32 {} //~ ERROR auto traits must not contain where bounds | |||
default impl !MyTrait for u32 {} | |||
//~^ ERROR negative impls on primitive types must not contain where bounds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the "where bound" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems like it's more about the default
keyword, perhaps?
|
||
trait MyTrait {} | ||
|
||
impl<T> !MyTrait for T {} //~ ERROR auto traits must not contain where bounds | ||
impl<T> !MyTrait for T {} | ||
//~^ ERROR negative impls on type parameters must not contain where bounds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any way to highlight the T: Sized
bound? e.g., for drop we print something like
error[E0367]:
Drop
impl requiresT: std::marker::Sized
but the struct it is implemented for does not
This is still wip and I think I should reuse the drop code even if the self type is not an adt. Will ping you on zulip once it makes sense to review this again 😅 |
☔ The latest upstream changes (presumably #75549) made this pull request unmergeable. Please resolve the merge conflicts. |
let's do this at another time before stabilizing negative impls. This is not really important rn |
closes #74629
r? @nikomatsakis