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 needless_lifetimes inside macros (needs test!) #5283

Closed
matthiaskrgr opened this issue Mar 7, 2020 · 4 comments · Fixed by #5293 or #10257
Closed

skip needless_lifetimes inside macros (needs test!) #5283

matthiaskrgr opened this issue Mar 7, 2020 · 4 comments · Fixed by #5293 or #10257
Assignees
Labels
E-needs-test Call for participation: writing tests T-macros Type: Issues with macros and macro expansion

Comments

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Mar 7, 2020

I came across this example

warning: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration)
    --> src/librustc/ty/query/plumbing.rs:1156:19
     |
817  | /        macro_rules! define_queries {
818  | |            (<$tcx:tt> $($category:tt {
819  | |                $($(#[$attr:meta])* [$($modifiers:tt)*] fn $name:ident: $node:ident($K:ty) -> $V:ty,)*
820  | |            },)*) => {
821  | /                define_queries_inner! { <$tcx>
822  |                      $($( $(#[$attr])* category<$category> [$($modifiers)*] fn $name: $node($K) -> $V,)*)*
823  | |                }
     | |________________- in this macro invocation (#3)
824  | |            }
825  | |        }
     | |________- in this expansion of `define_queries!` (#2)
826  | 
827  | /        macro_rules! define_queries_inner {
828  | |            (<$tcx:tt>
829  | |             $($(#[$attr:meta])* category<$category:tt>
830  | |                [$($modifiers:tt)*] fn $name:ident: $node:ident($K:ty) -> $V:ty,)*) => {
...    |
1118 | /                define_provider_struct! {
1119 |                      tcx: $tcx,
1120 |                      input: ($(([$($modifiers)*] [$name] [$K] [$V]))*)
1121 | |                }
     | |________________- in this macro invocation (#4)
...
1127 | |            }
1128 | |        }
     | |________- in this expansion of `define_queries_inner!` (#3)
...
1147 | /        macro_rules! define_provider_struct {
1148 | |            (tcx: $tcx:tt,
1149 | |             input: ($(([$($modifiers:tt)*] [$name:ident] [$K:ty] [$R:ty]))*)) => {
1150 | |                pub struct Providers<$tcx> {
...    |
1156 |                          $(fn $name<$tcx>(_: TyCtxt<$tcx>, key: $K) -> $R {
     |  __________________________^
1157 |                              bug!("`tcx.{}({:?})` unsupported by its crate",
1158 |                                   stringify!($name), key);
1159 | |                        })*
     | |________________________^
...
1163 | |            };
1164 | |        }
     | |________- in this expansion of `define_provider_struct!` (#4)
     | 
    ::: src/librustc/ty/query/mod.rs:107:1
     |
107  | /        rustc_query_append! { [define_queries!][ <'tcx>
108  | |            Other {
109  | |                /// Runs analysis passes on the crate.
110  | |                [eval_always] fn analysis: Analysis(CrateNum) -> Result<(), ErrorReported>,
111  | |            },
112  | |        ]}
     | |_________- in this macro invocation (#1)
     | 
    ::: src/librustc/query/mod.rs:38:1
     |
38   |        / rustc_queries! {
39   |        |     Other {
40   |        |         query trigger_delay_span_bug(key: DefId) -> () {
41   |        |             desc { "trigger a delay span bug" }
...           |
1254 |        |     }
1255 |        | }
     |        | -
     |        | |
     |        |_in this expansion of `rustc_query_append!` (#1)
     |          in this macro invocation (#2)
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes

and I'm wondering if the lint should skip macros.

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy T-macros Type: Issues with macros and macro expansion labels Mar 9, 2020
@matthiaskrgr
Copy link
Member Author

Hmm, actually the lint should be skipped in external macros but not in local ones...?

    if in_external_macro(cx.sess(), span) || has_where_lifetimes(cx, &generics.where_clause) {
        return;
    }

/~https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/lifetimes.rs#L129

matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this issue Mar 9, 2020
@matthiaskrgr matthiaskrgr added E-needs-test Call for participation: writing tests and removed good-first-issue These issues are a good way to get started with Clippy labels Mar 30, 2020
bors added a commit that referenced this issue Mar 30, 2020
don't emit lifetime lint warnings for code inside macros.

Fixes #5283

changelog: Don't emit lifetime lint warnings for code inside macros.
@bors bors closed this as completed in 8e83afa Mar 30, 2020
@matthiaskrgr matthiaskrgr reopened this Mar 30, 2020
@matthiaskrgr
Copy link
Member Author

Reopened as this still needs a testcase (see #5293 (comment))

@matthiaskrgr matthiaskrgr changed the title skip needless_lifetimes inside macros? skip needless_lifetimes inside macros (needs test!) Mar 30, 2020
@tylerjw
Copy link
Contributor

tylerjw commented Jan 30, 2023

@rustbot claim

@matthiaskrgr I created a PR to add a test for this. I see your comment above about external macros, but I need to figure out how to test that. Do you mean a macro defined in a different crate?

Similarly, I have this PR, which could use something defined in a separate crate for testing, and I am unsure how to achieve that. Are there any example tests in Clippy that define symbols in external crates?

@tylerjw
Copy link
Contributor

tylerjw commented Jan 30, 2023

@matthiaskrgr I updated the PR to lint local macros and test for local and external macro expansion with needless lifetimes.

@bors bors closed this as completed in e1224cd Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: writing tests T-macros Type: Issues with macros and macro expansion
Projects
None yet
3 participants