rustc_resolve: improve const generic errors#152913
rustc_resolve: improve const generic errors#152913Unique-Usman wants to merge 1 commit intorust-lang:mainfrom
Conversation
|
rustbot has assigned @JonathanBrouwer. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? @estebank |
00fa325 to
98d6219
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
98d6219 to
b162b4b
Compare
| .struct_constructors | ||
| .insert(local_def_id, (ctor_res, ctor_vis.to_def_id(), ret_fields)); | ||
| } | ||
| self.r.struct_generics.insert(local_def_id, generics.clone()); |
There was a problem hiding this comment.
Let's see if we can struct_generics store references. That should keep mem usage to a minimum.
| if let Some(const_err) = const_err { | ||
| err.cancel(); | ||
| err = const_err; | ||
| } |
There was a problem hiding this comment.
This is currently inactive, right? const_err isn't None anywhere right now?
There was a problem hiding this comment.
const_err is actually always None except in the case of where we add the error we just introduce see it here -> https://github.com/rust-lang/rust/pull/152913/changes#diff-2c81e6d8048a31a3ce43f30181aaa037e0ad0076e7aee67792ca993f28c75ac3R3440-R3441
There was a problem hiding this comment.
it is to prevent the type error from showing up.
|
HIR ty lowering was modified cc @fmease |
This comment has been minimized.
This comment has been minimized.
b162b4b to
42de5af
Compare
This comment has been minimized.
This comment has been minimized.
42de5af to
aa46052
Compare
| @@ -4,18 +4,19 @@ error[E0575]: expected associated type, found associated constant `Trait::ASSOC` | |||
| LL | bar::<<T as Trait>::ASSOC>(); | |||
| | ^^^^^^^^^^^^^^^^^^^ not a associated type | |||
|
|
|||
| error[E0747]: unresolved item provided when a constant was expected | |||
| --> $DIR/assoc_const_as_type_argument.rs:8:11 | |||
| error[E0284]: type annotations needed | |||
There was a problem hiding this comment.
It is unfortunate that after silencing E0747 we end up with these inference errors instead...
I think we can split the work into two PRs: one silencing the unnecessary/redundant resolve error ("unresolved item when X was expected") and another for the appropriate const generic suggestions. That would make things easier to review and evolve on isolation.
There was a problem hiding this comment.
Yeah, thanks. we can resolve the E0747 error after getting this pr merged. I already update the patch to not silence the E0747 for now.
|
Lets see what impact keeping track of all struct @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rustc_resolve: improve const generic errors
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (94de249): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (secondary 0.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 480.488s -> 481.825s (0.28%) |
aa46052 to
6cbf656
Compare
There was a problem hiding this comment.
Will continue reading the logic in diagnostics.rs.
I tried today to silence the errors. I'd found one way that works for almost every case, except one where an outer const parameter is used in a nested item, which ICEs, so I'll look for another alternative.
| --> $DIR/invalid-const-arguments.rs:14:32 | ||
| | | ||
| LL | struct A<const N: u8>; | ||
| | ---------------------- similarly named struct `A` defined here | ||
| ... | ||
| LL | struct C<const C: u8, const N: u8>; | ||
| | ----------- const parameter `T` is defined on the type | ||
| LL | impl<const N: u8> Foo for C<N, T> {} | ||
| | ^ | ||
| | ^ not found in this scope |
There was a problem hiding this comment.
This reads a bit weirdly because the name of the const in the struct is different than on the impl. Let's think about how we can make it clearer, specially in the first span label.
There was a problem hiding this comment.
we can say something like:
the corresponding const parameter is defined here on the type
what do you think ?
There was a problem hiding this comment.
corresponding const parameter on the type defined here sounds good
6cbf656 to
144113a
Compare
This comment has been minimized.
This comment has been minimized.
144113a to
288fb83
Compare
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
288fb83 to
c83c8ff
Compare
No description provided.