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

skip dyn keyword lint under macros #59463

Merged

Commits on Mar 27, 2019

  1. Do not lint dyn tokens under macros.

    The existing `KeywordIdents` lint blindly scans the token stream for a
    macro or macro definition. It does not attempt to parse the input,
    which means it cannot distinguish between occurrences of `dyn` that
    are truly instances of it as an identifier (e.g. `let dyn = 3;`)
    versus occurrences that follow its usage as a contextual keyword (e.g.
    the type `Box<dyn Trait>`).
    
    In an ideal world the lint would parse the token stream in order to
    distinguish such occurrences; but in general we cannot do this,
    because a macro_rules definition does not specify what parsing
    contexts the macro being defined is allowed to be used within.
    
    So rather than put a lot of work into attempting to come up with a
    more precise but still incomplete solution, I am just taking the short
    cut of not linting any instance of `dyn` under a macro. This prevents
    `rustfix` from injecting bugs into legal 2015 edition code.
    pnkfelix committed Mar 27, 2019
    Configuration menu
    Copy the full SHA
    6046f4a View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    6d7e5df View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    1f63a52 View commit details
    Browse the repository at this point in the history

Commits on Mar 28, 2019

  1. Revise test slightly so that dyn in macro invocation *must* be pars…

    …ed as keyword in test.
    
    Back-story: After reflection this morning, I realized that the
    previous form of this test would allow the macro invocation to treat
    the `dyn` input as a raw-identifier rather than a keyword, and since
    the input was discarded by that version of the macro, the test would
    pass despite the detail that the input `dyn` should not have been
    parsed as a raw-identifier.
    
    This revision fixes that oversight, by actually *using* the macro
    input to construct a `Box<dyn Trait>` type.
    pnkfelix committed Mar 28, 2019
    Configuration menu
    Copy the full SHA
    f043d2d View commit details
    Browse the repository at this point in the history
  2. Revise and generalize the macros-unlinted tests.

    Review feedback asked for the test to be generalized to include macros
    2.0; that generalization is dyn-2015-idents-in-decl-macros-unlinted.rs
    
    As a drive-by, I also decided to revise the test to make it clear
    *why* we cannot generally lint these cases. (I already had similar
    demonstrations in dyn-2015-edition-keyword-ident-lint.rs, but it does
    not hurt to try to emphasize matters.)
    
    I also added some commentary on the cases where we could choose to
    make the lint smarter, namely the situations where a macro is
    *definitely* using `dyn` as an identifier (because it is using it as a
    path component).
    pnkfelix committed Mar 28, 2019
    Configuration menu
    Copy the full SHA
    528366d View commit details
    Browse the repository at this point in the history