-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Provide additional context to errors involving const traits #144194
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
base: master
Are you sure you want to change the base?
Conversation
Some changes occurred to constck cc @fee1-dead Some changes occurred to the CTFE machinery This PR modifies cc @jieyouxu |
When encountering an unmet `Ty: [const] Trait` bound, if `Trait` is `#[const_trait]` and there's an `impl Trait for Ty` point at it. If local, suggest `impl const Trait for Ty`, otherwise just point at it. ``` error[E0277]: the trait bound `NonConstAdd: [const] Add` is not satisfied --> $DIR/assoc-type.rs:37:16 | LL | type Bar = NonConstAdd; | ^^^^^^^^^^^ | note: required by a bound in `Foo::Bar` --> $DIR/assoc-type.rs:33:15 | LL | type Bar: [const] Add; | ^^^^^^^^^^^ required by this bound in `Foo::Bar` help: make the `impl` of trait `Add` `const` | LL | impl const Add for NonConstAdd { | +++++ ``` ``` error[E0277]: the trait bound `T: [const] PartialEq` is not satisfied --> tests/ui/traits/const-traits/call-generic-method-fail.rs:5:5 | 5 | *t == *t | ^^^^^^^^ | note: trait `PartialEq` is implemented but not `const` --> /home/gh-estebank/rust/library/core/src/ptr/const_ptr.rs:1590:1 | 1590 | impl<T: PointeeSized> PartialEq for *const T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: trait `PartialEq` is implemented but not `const` --> /home/gh-estebank/rust/library/core/src/ptr/mut_ptr.rs:2011:1 | 2011 | impl<T: PointeeSized> PartialEq for *mut T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ```
Point at trait and associated item when that associated item is used in a const context. Suggest making the trait `#[const_trait]`. ``` error[E0015]: cannot call non-const method `<() as Trait>::foo` in constant functions --> $DIR/inline-incorrect-early-bound-in-ctfe.rs:26:8 | LL | ().foo(); | ^^^^^ | note: method `foo` is not const because trait `Trait` is not const --> $DIR/inline-incorrect-early-bound-in-ctfe.rs:13:1 | LL | trait Trait { | ^^^^^^^^^^^ this trait is not const LL | fn foo(self); | ------------- this method is not const = note: calls in constant functions are limited to constant functions, tuple structs and tuple variants help: consider making trait `Trait` const | LL + #[const_trait] LL | trait Trait { | ```
``` error[E0015]: cannot call non-const associated function `Foo::{constant#0}::Foo::<17>::value` in constants --> $DIR/nested-type.rs:15:5 | LL | struct Foo<const N: [u8; { | __________________________- LL | | struct Foo<const N: usize>; LL | | LL | | impl<const N: usize> Foo<N> { ... | LL | | Foo::<17>::value() | | ^^^^^^^^^^^^^^^^^^ LL | | LL | | }]>; | |_- calls in constants are limited to constant functions, tuple structs and tuple variants ```
note: trait `PartialEq` is implemented but not `const` | ||
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | ||
note: trait `PartialEq` is implemented but not `const` | ||
--> $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL |
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.
Why is it pointing at the raw ptr impls?
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 that they are incorrectly detected as CandidateSimilarity::Exact
, which is incorrect. I can skip them explicitly on this PR by adding some other check, or look into it at another time.
trait_span.shrink_to_lo(), | ||
format!("consider making trait `{trait_name}` const"), | ||
format!("#[const_trait]\n{indentation}"), | ||
Applicability::MachineApplicable, |
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 not MachineApplicable
, since it doesn't fix the bug completely.
)); | ||
note = false; | ||
let def_kind = ccx.tcx.def_kind(callee); | ||
if let DefKind::AssocTy | DefKind::AssocConst | DefKind::AssocFn = def_kind { |
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.
Callees are always AssocFn
s, why did you add DefKind::AssocTy | DefKind::AssocConst
too?
if let DefKind::AssocTy | DefKind::AssocConst | DefKind::AssocFn = def_kind { | ||
let parent = ccx.tcx.parent(callee); | ||
if let DefKind::Trait = ccx.tcx.def_kind(parent) |
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 whole check could be simplified if you looked at opt_associated_item
and checked both the AssocKind
and the AssocItemContainer
, rather than using manual parenting logic.
non_or_conditionally, | ||
}); | ||
let context_span = ccx.tcx.def_span(ccx.def_id()); | ||
err.span_label(context_span, format!( |
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.
Why did you change this to a label only for this case, and keep it as a note for the rest? Having special logic changing it between a note and a label seems too special cased. I'd prefer if you reverted 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.
This line is pretty long for a label too.
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 wanted to point at the constant context in the output in some way, as it isn't always trivially obvious why a constant function was expected. As we already had a note talking about the const context I moved that same message over from the note to the label so we wouldn't need both.
It was
error[E0015]: cannot call non-const method `<T as Foo>::a` in constant functions
--> const-super-trait.rs:10:7
|
LL | const fn foo<T: ~const Bar>(x: &T) {
| ---------------------------------- calls in constant functions are limited to constant functions, tuple structs and tuple variants
LL | x.a();
| ^^^
vs
error[E0015]: cannot call non-const method `<T as Foo>::a` in constant functions
--> const-super-trait.rs:10:7
|
LL | x.a();
| ^^^
|
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
We could instead have
error[E0015]: cannot call non-const method `<T as Foo>::a` in constant functions
--> const-super-trait.rs:10:7
|
LL | const fn foo<T: ~const Bar>(x: &T) {
| ---------------------------------- required by this constant function
LL | x.a();
| ^^^
|
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
I was just trying to keep the verbosity increase to a minimum.
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 wanted to point at the constant context in the output in some way, as it isn't always trivially obvious why a constant function was expected.
I don't think the users need this to be a label. There's a tradeoff here with verbosity and I think this both complicates the implementation by having that note
boolean and also splits the presentation of the diagnostic.
When encountering an unmet
Ty: [const] Trait
bound, ifTrait
is#[const_trait]
and there's animpl Trait for Ty
point at it. If local, suggestimpl const Trait for Ty
, otherwise just point at it.Point at trait and associated item when that associated item is used in a const context. Suggest making the trait
#[const_trait]
. Point at theconst
context where theconst
expression was found.