Improve errors for recursive type aliases #88121
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
|
r? @estebank (feel free to reassign, but I know you were involved with that issue) |
| @@ -591,10 +591,14 @@ pub(crate) fn report_cycle<'a>( | |||
| err.span_note(span, &format!("...which requires {}...", query.description)); | |||
| } | |||
estebank
Aug 18, 2021
Contributor
I'm intrigued if the iteration above doesn't need to be reverted to be correct 🤔 (no need to address now)
I'm intrigued if the iteration above doesn't need to be reverted to be correct
|
I'm ok with the approach. Do you want to try out to expand the query with the |
I tried doing that but then realized that I'd need to execute a query (to get the I could move |
I tried doing that, but that requires moving a bunch of other things, like An alternative approach could be to just track a boolean that says whether something is a type alias or not; it's also somewhat hacky. Yet another approach could be to have a mini- |
Ok, that worked! I'll probably finish up and then push up my changes tomorrow or this weekend. |
camelid
force-pushed the
camelid:better-recursive-alias-error
branch
from
2c7d13b
to
568eb9c
|
I'm working on getting it working for trait aliases too, and then I think this should be ready for review. |
This comment has been hidden.
This comment has been hidden.
|
Once we think the logic is finished, we should do a perf run as you suggested earlier. |
This comment has been hidden.
This comment has been hidden.
camelid
force-pushed the
camelid:better-recursive-alias-error
branch
from
0e0788a
to
02529bb
camelid
force-pushed the
camelid:better-recursive-alias-error
branch
from
02529bb
to
e9f6e4b
|
Rebased (just to keep this up to date). |
|
r=me. The only thing I'd want us to have is the docstring on the new enum. Feel free to ignore the other drive by comments. |
| error[E0391]: cycle detected when computing the super predicates of `T1` | ||
| --> $DIR/infinite-trait-alias-recursion.rs:3:1 | ||
| | | ||
| LL | trait T1 = T2; | ||
| | ^^^^^^^^^^^^^^ |
estebank
Aug 27, 2021
Contributor
This should likely point at only T1.
This should likely point at only T1.
| note: cycle used when collecting item types in top-level module | ||
| --> $DIR/infinite-type-alias-mutual-recursion.rs:1:1 | ||
| | | ||
| LL | / type X1 = X2; | ||
| LL | | | ||
| LL | | type X2 = X3; | ||
| LL | | type X3 = X1; | ||
| LL | | | ||
| LL | | fn main() {} | ||
| | |____________^ |
estebank
Aug 27, 2021
Contributor
This reminds me, we need a way to talk about files without showing a snippet.
This reminds me, we need a way to talk about files without showing a snippet.
camelid
force-pushed the
camelid:better-recursive-alias-error
branch
from
e9f6e4b
to
c3df832
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
… r=<try> Improve errors for recursive type aliases Fixes rust-lang#17539.
|
|
|
Queued fb1a092 with parent dfd8472, future comparison URL. |
|
Finished benchmarking try commit (fb1a092): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. @bors rollup=never |
|
Perf looks good, I just have one question, and then this should be ready to be merged! |
`tcx.def_kind()` could theoretically invoke another query, which could cause an infinite query loop. Accessing the HIR map directly makes that less likely to happen. I also changed it to use `as_local()` (`tcx.def_kind()` seems to implicitly call `expect_local()`) and `opt_def_kind()` to reduce the chance of panicking on valid code.
|
Ok, updated it to use the HIR map directly! |
|
@bors r+ |
|
|
|
|
Fixes #17539.