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

Rust diagnostic for impl Trait suggests broken code #97760

Closed
jonasbb opened this issue Jun 5, 2022 · 1 comment · Fixed by #97778
Closed

Rust diagnostic for impl Trait suggests broken code #97760

jonasbb opened this issue Jun 5, 2022 · 1 comment · Fixed by #97778
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jonasbb
Copy link
Contributor

jonasbb commented Jun 5, 2022

Given the following code:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=87a0987f6c8b7b1a67b03338e2a4ed09
The issues also reproduces with the nightly version on the playground.

pub fn print_values(values: &impl IntoIterator)
where
{
    for x in values.into_iter() {
        println!("{x}");
    }
}

The current output is:

error[[E0277]](https://doc.rust-lang.org/stable/error-index.html#E0277): `<impl IntoIterator as IntoIterator>::Item` doesn't implement `std::fmt::Display`
 --> src/lib.rs:5:20
  |
5 |         println!("{x}");
  |                    ^ `<impl IntoIterator as IntoIterator>::Item` cannot be formatted with the default formatter
  |
  = help: the trait `std::fmt::Display` is not implemented for `<impl IntoIterator as IntoIterator>::Item`
  = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
  = note: this error originates in the macro `$crate::format_args_nl` (in Nightly builds, run with -Z macro-backtrace for more info)
help: introduce a type parameter with a trait bound instead of using `impl Trait`
  |
1 ~ pub fn print_values<I: IntoIterator>(values: &impl IntoIterator)
2 ~ where where <I as IntoIterator>::Item: std::fmt::Display
  |

The proposed code has multiple problems.
The code doubles the where leading to broken Rust.
The diagnostic correctly introduces the I: IntoIterator, but does not replace the impl IntoIterator in the function arguments.

If you remove the where from the initial example, the diagnostic changes to include +++ markers for the added code. These seem helpful and to me it is not clear why they are missing initially.

help: introduce a type parameter with a trait bound instead of using `impl Trait`
  |
1 | pub fn print_values<I: IntoIterator>(values: &impl IntoIterator) where <I as IntoIterator>::Item: std::fmt::Display
  |                    +++++++++++++++++                             ++++++++++++++++++++++++++++++++++++++++++++++++++
@jonasbb jonasbb added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 5, 2022
@compiler-errors
Copy link
Member

Working on this.

@compiler-errors compiler-errors self-assigned this Jun 5, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jun 11, 2022
…dy, r=cjgillot

Tidy up miscellaneous bounds suggestions

Just some small fixes to suggestions

- Generalizes `Ty::is_suggestable` into a `TypeVisitor`, so that it can be called on things other than `Ty`
- Makes `impl Trait` in arg position no longer suggestible (generalizing the fix in rust-lang#97640)
- Fixes `impl Trait` not being replaced with fresh type param when it's deeply nested in function signature (fixes rust-lang#97760)
- Fixes some poor handling of `where` clauses with no predicates (also rust-lang#97760)
- Uses `InferCtxt::resolve_numeric_literals_with_default` so we suggest `i32` instead of `{integer}` (fixes rust-lang#97677)

Sorry there aren't many tests the fixes. Most of them would just be duplicates of other tests with empty `where` clauses or `impl Trait` in arg position instead of generic params. Let me know if you'd want more test coverage.
@bors bors closed this as completed in 37a4225 Jun 12, 2022
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 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.

2 participants