Mention that type parameters are used recursively on bivariance error #127871

compiler-errors · 2024-07-17T20:02:23Z

Right now when a type parameter is used recursively, even with indirection (so it has a finite size) we say that the type parameter is unused:

struct B<T>(Box<B<T>>);

This is confusing, because the type parameter is used, it just doesn't have its variance constrained. This PR tweaks that message to mention that it must be used non-recursively.

Not sure if we should actually mention "variance" here, but also I'd somewhat prefer we don't keep the power users in the dark w.r.t the real underlying issue, which is that the variance isn't constrained. That technical detail is reserved for a note, though.

cc @fee1-dead

Fixes #118976
Fixes #26283
Fixes #53191
Fixes #105740
Fixes #110466

rustbot · 2024-07-17T20:02:30Z

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

compiler-errors · 2024-07-17T20:19:33Z

tests/ui/variance/variance-unused-type-param.stderr

+   |               - type parameter must be used non-recursively in the definition
+LL |     Cons(Box<ListCell<T>>),
+   |                       ^


Notably, the primary span is now the usage, not the definition. We could put that back, I guess.

fmease

Pretty sure this fixes #118976 (edit: Well, I marked that as A-docs not A-diagnostics but doesn't matter as long as we clear up the confusion somehow). Might also fix some of the open issues linked here: #118976 (comment) (dunno, haven't looked at them lol)

compiler/rustc_hir_analysis/messages.ftl

compiler/rustc_hir_analysis/src/check/wfcheck.rs

compiler-errors · 2024-07-17T21:22:15Z

Interestingly, this actually regresses #66912...

struct Foo<T> {
    bar: Bar<T>,
}
struct Bar<T> {}

Maybe we should recurse into the structs/aliases to check for cycles 🤔

fmease · 2024-07-17T21:31:13Z

Interestingly, this actually regresses #66912.

Oh, that's a bummer. Makes sense looking at the current impl.

If you can make it work easily, I'd love to see this addressed. Note though that the cycles may have arbitrary periods.

compiler-errors · 2024-07-17T21:34:35Z

Sure, but never greater than O(number of structs in the crate) if we're only doing a purely def-id based traversal to eliminate cases that are obviously not cycles. I don't really care about the substitution corner cases.

compiler-errors · 2024-07-17T22:06:26Z

OK, should also "fix" #66912 by explaining that the parameter is likely unused in the containing type. Not totally certain if that's sufficient, but it should at least unregress the test.

estebank · 2024-07-18T20:12:50Z

tests/ui/traits/issue-105231.stderr

+error: type parameter `T` is only used recursively
+  --> $DIR/issue-105231.rs:2:15
   |
 LL | struct A<T>(B<T>);
-   |          ^ unused type parameter
+   |          -    ^
+   |          |
+   |          type parameter must be used non-recursively in the definition
   |
   = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData`
-   = help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead
+   = note: all type parameters must be used in a non-recursive way in order to constrain their variance


It would be ideal if we either expanded this diagnostic with some of the info from E0072, or if we silenced this one if E0072 was emitted (because it gives the "solution" already, making this one redundant).

estebank · 2024-07-18T20:13:06Z

@bors r+

bors · 2024-07-18T20:13:09Z

📌 Commit c02d0de has been approved by estebank

It is now in the queue for this repository.

fmease · 2024-07-18T22:25:03Z

Esteban, I'm not entirely sure why you r+'ed even tho I had assigned myself. Clearly, I had sth. I wanted to say. Anyway, I will post it as a PR instead.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#127418 (Wrap too long type name) - rust-lang#127594 (Fuchsia status code match arm) - rust-lang#127835 (Fix ICE in suggestion caused by `⩵` being recovered as `==`) - rust-lang#127858 (match lowering: Rename `MatchPair` to `MatchPairTree`) - rust-lang#127871 (Mention that type parameters are used recursively on bivariance error) - rust-lang#127913 (remove `debug-logging` default from tools profile) - rust-lang#127925 (Remove tag field from `Relation`s) - rust-lang#127929 (Use more accurate span for `addr_of!` suggestion) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 8 pull requests Successful merges: - rust-lang#127418 (Wrap too long type name) - rust-lang#127594 (Fuchsia status code match arm) - rust-lang#127835 (Fix ICE in suggestion caused by `⩵` being recovered as `==`) - rust-lang#127858 (match lowering: Rename `MatchPair` to `MatchPairTree`) - rust-lang#127871 (Mention that type parameters are used recursively on bivariance error) - rust-lang#127913 (remove `debug-logging` default from tools profile) - rust-lang#127925 (Remove tag field from `Relation`s) - rust-lang#127929 (Use more accurate span for `addr_of!` suggestion) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#127871 - compiler-errors:recursive, r=estebank Mention that type parameters are used recursively on bivariance error Right now when a type parameter is used recursively, even with indirection (so it has a finite size) we say that the type parameter is unused: ``` struct B<T>(Box<B<T>>); ``` This is confusing, because the type parameter is *used*, it just doesn't have its variance constrained. This PR tweaks that message to mention that it must be used *non-recursively*. Not sure if we should actually mention "variance" here, but also I'd somewhat prefer we don't keep the power users in the dark w.r.t the real underlying issue, which is that the variance isn't constrained. That technical detail is reserved for a note, though. cc `@fee1-dead` Fixes rust-lang#118976 Fixes rust-lang#26283 Fixes rust-lang#53191 Fixes rust-lang#105740 Fixes rust-lang#110466

…better-err, r=compiler-errors Lazy type aliases: Diagostics: Detect bivariant ty params that are only used recursively Follow-up to errs's rust-lang#127871. Extends the logic to cover LTAs, too, not just ADTs. This change only takes effect with the next-gen solver enabled as cycle errors like the one we have here are fatal in the old solver. That's my explanation anyways. r? compiler-errors

Rollup merge of rust-lang#127976 - fmease:lta-cyclic-bivariant-param-better-err, r=compiler-errors Lazy type aliases: Diagostics: Detect bivariant ty params that are only used recursively Follow-up to errs's rust-lang#127871. Extends the logic to cover LTAs, too, not just ADTs. This change only takes effect with the next-gen solver enabled as cycle errors like the one we have here are fatal in the old solver. That's my explanation anyways. r? compiler-errors

Mention that type parameters are used recursively

3716a3f

rustbot assigned estebank Jul 17, 2024

compiler-errors changed the title ~~Mention that type parameters are used recursively~~ Mention that type parameters are used recursively on bivariance error Jul 17, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 17, 2024

compiler-errors commented Jul 17, 2024

View reviewed changes

fmease reviewed Jul 17, 2024

View reviewed changes

compiler/rustc_hir_analysis/messages.ftl Outdated Show resolved Hide resolved

compiler-errors commented Jul 17, 2024

View reviewed changes

compiler/rustc_hir_analysis/src/check/wfcheck.rs Show resolved Hide resolved

Account for self ty alias

a0a251a

This comment has been minimized.

Sign in to view

Account for structs that have unused params in nested types in fields

c02d0de

compiler-errors force-pushed the recursive branch from dfd6084 to c02d0de Compare July 18, 2024 01:12

fmease assigned fmease and unassigned estebank Jul 18, 2024

estebank reviewed Jul 18, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2024

matthiaskrgr mentioned this pull request Jul 18, 2024

Rollup of 8 pull requests #127936

Merged

bors merged commit 65de5d0 into rust-lang:master Jul 19, 2024
6 checks passed

rustbot added this to the 1.81.0 milestone Jul 19, 2024

fmease mentioned this pull request Jul 19, 2024

Lazy type aliases: Diagnostics: Detect bivariant ty params that are only used recursively #127976

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mention that type parameters are used recursively on bivariance error #127871

Mention that type parameters are used recursively on bivariance error #127871

compiler-errors commented Jul 17, 2024 •

edited

Loading

rustbot commented Jul 17, 2024

compiler-errors Jul 17, 2024

fmease left a comment •

edited

Loading

compiler-errors commented Jul 17, 2024 •

edited

Loading

fmease commented Jul 17, 2024

compiler-errors commented Jul 17, 2024

compiler-errors commented Jul 17, 2024

This comment has been minimized.

estebank Jul 18, 2024

estebank commented Jul 18, 2024

bors commented Jul 18, 2024

fmease commented Jul 18, 2024

Mention that type parameters are used recursively on bivariance error #127871

Mention that type parameters are used recursively on bivariance error #127871

Conversation

compiler-errors commented Jul 17, 2024 • edited Loading

rustbot commented Jul 17, 2024

compiler-errors Jul 17, 2024

Choose a reason for hiding this comment

fmease left a comment • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Jul 17, 2024 • edited Loading

fmease commented Jul 17, 2024

compiler-errors commented Jul 17, 2024

compiler-errors commented Jul 17, 2024

This comment has been minimized.

estebank Jul 18, 2024

Choose a reason for hiding this comment

estebank commented Jul 18, 2024

bors commented Jul 18, 2024

fmease commented Jul 18, 2024

compiler-errors commented Jul 17, 2024 •

edited

Loading

fmease left a comment •

edited

Loading

compiler-errors commented Jul 17, 2024 •

edited

Loading