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

bare_trait_objects help is incorrect with Box<Trait + 'lifetime> in 2015 edition #63330

Open
mmitteregger opened this issue Aug 6, 2019 · 3 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mmitteregger
Copy link

mmitteregger commented Aug 6, 2019

The help text message for bare_trait_objects is incorrect when using the 2015 edition.

The following example gives a correct help text in the 2015 and 2018 edition:

pub fn test(_: Box<::std::any::Any>) {}
warning: trait objects without an explicit `dyn` are deprecated
 --> src/lib.rs:1:20
  |
1 | pub fn test(_: Box<::std::any::Any>) {
  |                    ^^^^^^^^^^^^^^^ help: use `dyn`: `dyn (::std::any::Any)`
  |
  = note: `#[warn(bare_trait_objects)]` on by default

but adding a lifetime to the trait bound produces an incorrect help message for the 2015 edition:

pub fn test(_: Box<::std::any::Any + 'static>) {
}
warning: trait objects without an explicit `dyn` are deprecated
 --> src/lib.rs:1:20
  |
1 | pub fn test(_: Box<::std::any::Any + 'static>) {
  |                    ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `dyn`: `dyn ::std::any::Any + 'static`
  |
  = note: `#[warn(bare_trait_objects)]` on by default

Applying the suggest help text in 2015 edition code results in:

pub fn test(_: Box<dyn ::std::any::Any + 'static>) {
}
error[E0433]: failed to resolve: use of undeclared type or module `dyn`
 --> src/lib.rs:1:20
  |
1 | pub fn test(_: Box<dyn ::std::any::Any + 'static>) {
  |                    ^^^ use of undeclared type or module `dyn`

warning: trait objects without an explicit `dyn` are deprecated
 --> src/lib.rs:1:20
  |
1 | pub fn test(_: Box<dyn ::std::any::Any + 'static>) {
  |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `dyn`: `dyn dyn ::std::any::Any + 'static`
  |
  = note: `#[warn(bare_trait_objects)]` on by default

error: aborting due to previous error

For more information about this error, try `rustc --explain E0433`.
error: Could not compile `playground`.

To learn more, run the command again with --verbose.

The code actually compiles with the 2018 edition.
The correct fix for 2015 edition code (which also works for edition 2018) is probably to include parens:

pub fn test(_: Box<dyn (::std::any::Any) + 'static>) {
}

I see two options:

  1. Add parens to the help text so that it is correct in both editions.
  2. Fix the help text only for the 2015 edition, because the suggested fix is already correct for the 2018 edition.

I personally prefer the first option for consistency with the existing help text and easier copy/pasting between editions.

Meta

rustc --version --verbose:

rustc 1.38.0-nightly (c4715198b 2019-08-05)
binary: rustc
commit-hash: c4715198b50d1cdaad44b6e250844362b77dcdd7
commit-date: 2019-08-05
host: x86_64-pc-windows-gnu
release: 1.38.0-nightly
LLVM version: 9.0
@jonas-schievink jonas-schievink added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 6, 2019
@Centril
Copy link
Contributor

Centril commented Aug 6, 2019

The issue here is that dyn unfortunately is not a proper keyword but only contextual on the 2015 edition. I think the fix is for the lint to account for those situations in the 2015 edition and only insert parenthesis as needed.

@ehuss
Copy link
Contributor

ehuss commented Sep 10, 2019

This also trips up edition transitions. Using cargo fix --edition will fail (see rust-lang/cargo#7349). If the suggestion is changed to add parentheses, then you'll end up with unnecessary parentheses in 2018. Perhaps rustfmt could strip them afterwards, though. (Or maybe a clippy warning?)

@estebank estebank added the D-edition Diagnostics: An error or lint that should account for edition differences. label Oct 5, 2019
@joshtriplett
Copy link
Member

I ran into this as well, when patching a crate that uses the 2015 edition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants