Skip to content
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

Recursive types in a Vec<T> with generic parameters gives ambiguous error #110466

Closed
Kjolnyr opened this issue Apr 17, 2023 · 5 comments · Fixed by #127871
Closed

Recursive types in a Vec<T> with generic parameters gives ambiguous error #110466

Kjolnyr opened this issue Apr 17, 2023 · 5 comments · Fixed by #127871
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-variance Area: Variance (https://doc.rust-lang.org/nomicon/subtyping.html) D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Kjolnyr
Copy link

Kjolnyr commented Apr 17, 2023

I tried this code:

struct Foo<T> { v: Vec<Bar<T>> }
struct Bar<T> { f: Foo<T> }

I expected to see this happen:

error[[E0072]](https://doc.rust-lang.org/stable/error_codes/E0072.html): recursive types `Foo` and `Bar` have infinite size
 --> src/lib.rs:1:1
  |
1 | struct Foo<T> { v: Bar<T> }
  | ^^^^^^^^^^^^^      ------ recursive without indirection
2 | struct Bar<T> { f: Foo<T> }
  | ^^^^^^^^^^^^^      ------ recursive without indirection
  |
help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle

OR actually compiling.

Instead, this happened: not compiling

error[E0392]: parameter `T` is never used
 --> src/main.rs:2:12
  |
2 | struct Foo<T> { v: Vec<Bar<T>> }
  |            ^ unused parameter
  |
  = 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: usize` instead

error[E0392]: parameter `T` is never used
 --> src/main.rs:3:12
  |
3 | struct Bar<T> { f: Foo<T> }
  |            ^ unused parameter
  |
  = 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: usize` instead

For more information about this error, try `rustc --explain E0392`.

Meta

rustc --version --verbose:

rustc 1.70.0-nightly (db0cbc48d 2023-03-26)
binary: rustc
commit-hash: db0cbc48d4aaa300713a95d9b317a365a474490c
commit-date: 2023-03-26
host: x86_64-pc-windows-msvc
release: 1.70.0-nightly
LLVM version: 16.0.0

@Kjolnyr Kjolnyr added the C-bug Category: This is a bug. label Apr 17, 2023
@Ezrashaw
Copy link
Contributor

Ezrashaw commented Apr 18, 2023

This should compile right? There is indirection because Vec<T> is heap-allocated.

@rustbot claim
(not sure I'll be able to fix it)

EDIT: rustbot doesn't like me lol

@rustbot
Copy link
Collaborator

rustbot commented Apr 18, 2023

Error: Parsing assign command in comment failed: ...' claim' | error: expected end of command at >| ' (not sure'...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@zirconium-n
Copy link
Contributor

This is not a bug. T is unused, and it's impossible to derive variance for T.

Simpler example:

// I "inlined" `Foo` and replaced `Vec` with `Box`. 
struct Bar<T> {
    f: Box<Bar<T>>,
}

It's easy to replace inner Bar<T> with Self to see T is unused.

// I "inlined" `Foo` and replaced `Vec` with `Box`. 
struct Bar<T> {
    f: Box<Self>,
}

@Ezrashaw
Copy link
Contributor

Ezrashaw commented Apr 18, 2023

I think you are right. In that case, we need to emit an error when inferring variance and then not emit the "unused parameter" error in that case. EDIT: Should this be a compile error or should we mark the type parameter as invariant?

I'm not entirely sure about the correctness of your reduction.

@Ezrashaw
Copy link
Contributor

@zirconium-n I think that'll delegate to someone more experienced than me for the final say on this:

https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/variance.20of.20mutually.20recursive.20ADTs

@fmease fmease added A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. A-variance Area: Variance (https://doc.rust-lang.org/nomicon/subtyping.html) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed C-bug Category: This is a bug. needs-triage-legacy labels Jan 29, 2024
@bors bors closed this as completed in 65de5d0 Jul 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 19, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-variance Area: Variance (https://doc.rust-lang.org/nomicon/subtyping.html) D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants