-
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
Constify PartialEq
#133995
base: master
Are you sure you want to change the base?
Constify PartialEq
#133995
Conversation
r? @wesleywiser rustbot has assigned @wesleywiser. Use |
r? @rust-lang/project-const-traits |
Probably want to gate these under a real gate given #133999 |
☔ The latest upstream changes (presumably #133522) made this pull request unmergeable. Please resolve the merge conflicts. |
@@ -1,12 +1,13 @@ | |||
error[E0015]: cannot match on `str` in constant functions | |||
--> $DIR/match-non-const-eq.rs:7:9 | |||
error[E0658]: cannot call conditionally-const method `<str as PartialEq>::eq` in constant functions |
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.
Nitpick: surprised that when rendering predicates we talk about T: ~const PartialEq
, but in fully qualified paths we talk <str as PartialEq>::eq
instead of something like <str as ~const PartialEq>::eq
. We already mention "conditionally-const" in the text, but wonder how many diagnostics will have to account for that to properly convey that context.
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.
well, so, <str as ~const PartialEq>::eq
is not a valid qpath. the method itself still is just <str as PartialEq>::eq
, it just has an additional "const requirement" on it.
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.
like, <str as ~const PartialEq>::eq
and <str as PartialEq>::eq
being distinct suggests that they are different methods, but they are not. this is a consequence of the current implementation of conditional constness.
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.
this is somewhat related to the fact that we allow users to write T: Trait<Assoc = i32>
, but not in method call position (i.e. <T as Trait<Assoc = i32>>::method
is not valid). The constness of a trait is associated with a trait impl. Imagine that T: ~const Trait
were written like T: Trait<constness = ~const>
.
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.
r=me with or without addressing the diagnostic regression
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.
The tracking issue #101871 has never been updated >.< It didn't track the unconstification.
I'd like to block this on #133999. |
Diagnostics regression due to the fact that if a trait is
#[const_trait]
we don't dispatch to the pretty complex logic inFnCallNonConst
.Perhaps we should just dispatch to that op if the feature gate is not enabled...... thoughts?