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

edition compatibility lints: Warning about dyn in a macro incorrectly #56327

Closed
alexcrichton opened this issue Nov 28, 2018 · 2 comments · Fixed by #59463
Closed

edition compatibility lints: Warning about dyn in a macro incorrectly #56327

alexcrichton opened this issue Nov 28, 2018 · 2 comments · Fixed by #59463
Assignees
Labels
A-edition-2018-lints Area: Lints supporting the 2018 edition A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

alexcrichton commented Nov 28, 2018

This code:

#![warn(rust_2018_compatibility)]

macro_rules! foo {
    () => {
        fn _foo() {
            let _x: Box<dyn Drop>;
        }
    }
}

foo!();

when compiled under 2015 edition yields:

warning: `dyn` is a keyword in the 2018 edition
 --> src/lib.rs:6:25
  |
6 |             let _x: Box<dyn Drop>;
  |                         ^^^ help: you can use a raw identifier to stay compatible: `r#dyn`
  |
note: lint level defined here
 --> src/lib.rs:1:9
  |
1 | #![warn(rust_2018_compatibility)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^
  = note: #[warn(keyword_idents)] implied by #[warn(rust_2018_compatibility)]
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
  = note: for more information, see issue #49716 <https://github.com/rust-lang/rust/issues/49716>

but the warning and suggestion are incorrect!

First reported at rust-lang/cargo#6359

This also applies to macro invocations

@alexcrichton alexcrichton added the A-edition-2018-lints Area: Lints supporting the 2018 edition label Nov 28, 2018
@alexcrichton alexcrichton added the A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` label Jan 7, 2019
@pnkfelix pnkfelix added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-nominated labels Mar 15, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Mar 18, 2019

(I nominated this for discussion at a future T-compiler meeting because I want us to figure out what our intent was for this bug, and I also want to try to develop a firmer policy for how such cases should be addressed going forward.)

@pnkfelix
Copy link
Member

discussed at T-compiler meeting. calling this P-medium. ( @nikomatsakis and I are not currently sure what our intentions are for dyn here.)

@pnkfelix is going to assign this to themself to see if there's any simple resolution focused on special casing behavior for dyn in macro bodies.

@pnkfelix pnkfelix added P-medium Medium priority and removed I-nominated labels Mar 21, 2019
@pnkfelix pnkfelix self-assigned this Mar 21, 2019
pnkfelix added a commit to pnkfelix/rust that referenced this issue Mar 27, 2019
Centril added a commit to Centril/rust that referenced this issue Mar 30, 2019
…rd-lint-under-macros, r=matthewjasper

skip dyn keyword lint under macros

This PR is following my own intuition that `rustfix` should never inject bugs into working code (even if that comes at the expense of it failing to fix things that will become bugs).

Fix rust-lang#56327
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2018-lints Area: Lints supporting the 2018 edition A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` P-medium Medium priority 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