Skip to content

Commit 676d71a

Browse files
committed
Defer is_from_proc_macro check and add tests
Move the is_from_proc_macro check inside check_fn_inner to be called only right before emitting lints, rather than at the entry points. This avoids the expensive check when early returns would prevent any lint emission anyway. Add tests for proc-macro generated code covering all check locations: - Standalone functions - Methods in impl blocks - Trait methods - Impl blocks with extra lifetimes
1 parent d7e5996 commit 676d71a

File tree

5 files changed

+118
-24
lines changed

5 files changed

+118
-24
lines changed

clippy_lints/src/lifetimes.rs

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,9 @@ impl<'tcx> LateLintPass<'tcx> for Lifetimes {
149149
..
150150
} = item.kind
151151
{
152-
if is_from_proc_macro(cx, item) {
153-
return;
154-
}
155-
check_fn_inner(cx, sig, Some(id), None, generics, item.span, true, self.msrv);
152+
check_fn_inner(cx, sig, Some(id), None, generics, item.span, true, self.msrv, || {
153+
is_from_proc_macro(cx, item)
154+
});
156155
} else if let ItemKind::Impl(impl_) = &item.kind
157156
&& !item.span.from_expansion()
158157
&& !is_from_proc_macro(cx, item)
@@ -162,9 +161,7 @@ impl<'tcx> LateLintPass<'tcx> for Lifetimes {
162161
}
163162

164163
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
165-
if let ImplItemKind::Fn(ref sig, id) = item.kind
166-
&& !is_from_proc_macro(cx, item)
167-
{
164+
if let ImplItemKind::Fn(ref sig, id) = item.kind {
168165
let report_extra_lifetimes = trait_ref_of_method(cx, item.owner_id).is_none();
169166
check_fn_inner(
170167
cx,
@@ -175,19 +172,28 @@ impl<'tcx> LateLintPass<'tcx> for Lifetimes {
175172
item.span,
176173
report_extra_lifetimes,
177174
self.msrv,
175+
|| is_from_proc_macro(cx, item),
178176
);
179177
}
180178
}
181179

182180
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
183-
if let TraitItemKind::Fn(ref sig, ref body) = item.kind
184-
&& !is_from_proc_macro(cx, item)
185-
{
181+
if let TraitItemKind::Fn(ref sig, ref body) = item.kind {
186182
let (body, trait_sig) = match *body {
187183
TraitFn::Required(sig) => (None, Some(sig)),
188184
TraitFn::Provided(id) => (Some(id), None),
189185
};
190-
check_fn_inner(cx, sig, body, trait_sig, item.generics, item.span, true, self.msrv);
186+
check_fn_inner(
187+
cx,
188+
sig,
189+
body,
190+
trait_sig,
191+
item.generics,
192+
item.span,
193+
true,
194+
self.msrv,
195+
|| is_from_proc_macro(cx, item),
196+
);
191197
}
192198
}
193199
}
@@ -202,6 +208,7 @@ fn check_fn_inner<'tcx>(
202208
span: Span,
203209
report_extra_lifetimes: bool,
204210
msrv: Msrv,
211+
is_from_proc_macro: impl FnOnce() -> bool,
205212
) {
206213
if span.in_external_macro(cx.sess().source_map()) || has_where_lifetimes(cx, generics) {
207214
return;
@@ -253,10 +260,19 @@ fn check_fn_inner<'tcx>(
253260
}
254261
}
255262

256-
if let Some((elidable_lts, usages)) = could_use_elision(cx, sig.decl, body, trait_sig, generics.params, msrv) {
257-
if usages.iter().any(|usage| !usage.ident.span.eq_ctxt(span)) {
258-
return;
259-
}
263+
let elidable = could_use_elision(cx, sig.decl, body, trait_sig, generics.params, msrv);
264+
let has_elidable_lts = elidable
265+
.as_ref()
266+
.is_some_and(|(_, usages)| !usages.iter().any(|usage| !usage.ident.span.eq_ctxt(span)));
267+
268+
// Only check is_from_proc_macro if we're about to emit a lint (it's an expensive check)
269+
if (has_elidable_lts || report_extra_lifetimes) && is_from_proc_macro() {
270+
return;
271+
}
272+
273+
if let Some((elidable_lts, usages)) = elidable
274+
&& has_elidable_lts
275+
{
260276
// async functions have usages whose spans point at the lifetime declaration which messes up
261277
// suggestions
262278
let include_suggestions = !sig.header.is_async();

tests/ui/extra_unused_lifetimes.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//@aux-build:proc_macro_derive.rs
2+
//@aux-build:proc_macros.rs
23

34
#![allow(
45
unused,
@@ -11,6 +12,7 @@
1112

1213
#[macro_use]
1314
extern crate proc_macro_derive;
15+
extern crate proc_macros;
1416

1517
fn empty() {}
1618

@@ -148,4 +150,34 @@ mod issue_13578 {
148150
impl<'a, T: 'a> Foo for Option<T> where &'a T: Foo {}
149151
}
150152

153+
// no lint on proc macro generated code
154+
mod proc_macro_generated {
155+
use proc_macros::external;
156+
157+
// no lint on external macro (extra unused lifetimes in impl block)
158+
external! {
159+
struct ExternalImplStruct;
160+
161+
impl<'a> ExternalImplStruct {
162+
fn foo() {}
163+
}
164+
}
165+
166+
// no lint on external macro (extra unused lifetimes in method)
167+
external! {
168+
struct ExternalMethodStruct;
169+
170+
impl ExternalMethodStruct {
171+
fn bar<'a>(&self) {}
172+
}
173+
}
174+
175+
// no lint on external macro (extra unused lifetimes in trait method)
176+
external! {
177+
trait ExternalUnusedLifetimeTrait {
178+
fn unused_lt<'a>(x: u8) {}
179+
}
180+
}
181+
}
182+
151183
fn main() {}

tests/ui/extra_unused_lifetimes.stderr

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this lifetime isn't used in the function definition
2-
--> tests/ui/extra_unused_lifetimes.rs:19:14
2+
--> tests/ui/extra_unused_lifetimes.rs:21:14
33
|
44
LL | fn unused_lt<'a>(x: u8) {}
55
| ^^
@@ -8,37 +8,37 @@ LL | fn unused_lt<'a>(x: u8) {}
88
= help: to override `-D warnings` add `#[allow(clippy::extra_unused_lifetimes)]`
99

1010
error: this lifetime isn't used in the function definition
11-
--> tests/ui/extra_unused_lifetimes.rs:47:10
11+
--> tests/ui/extra_unused_lifetimes.rs:49:10
1212
|
1313
LL | fn x<'a>(&self) {}
1414
| ^^
1515

1616
error: this lifetime isn't used in the function definition
17-
--> tests/ui/extra_unused_lifetimes.rs:74:22
17+
--> tests/ui/extra_unused_lifetimes.rs:76:22
1818
|
1919
LL | fn unused_lt<'a>(x: u8) {}
2020
| ^^
2121

2222
error: this lifetime isn't used in the impl
23-
--> tests/ui/extra_unused_lifetimes.rs:86:10
23+
--> tests/ui/extra_unused_lifetimes.rs:88:10
2424
|
2525
LL | impl<'a> std::ops::AddAssign<&Scalar> for &mut Scalar {
2626
| ^^
2727

2828
error: this lifetime isn't used in the impl
29-
--> tests/ui/extra_unused_lifetimes.rs:93:10
29+
--> tests/ui/extra_unused_lifetimes.rs:95:10
3030
|
3131
LL | impl<'b> Scalar {
3232
| ^^
3333

3434
error: this lifetime isn't used in the function definition
35-
--> tests/ui/extra_unused_lifetimes.rs:95:26
35+
--> tests/ui/extra_unused_lifetimes.rs:97:26
3636
|
3737
LL | pub fn something<'c>() -> Self {
3838
| ^^
3939

4040
error: this lifetime isn't used in the impl
41-
--> tests/ui/extra_unused_lifetimes.rs:125:10
41+
--> tests/ui/extra_unused_lifetimes.rs:127:10
4242
|
4343
LL | impl<'a, T: Source + ?Sized + 'a> Source for Box<T> {
4444
| ^^

tests/ui/needless_lifetimes.fixed

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,13 +470,36 @@ mod in_macro {
470470
}
471471
}
472472

473-
// no lint on external macro
473+
// no lint on external macro (standalone function)
474474
external! {
475475
fn needless_lifetime<'a>(x: &'a u8) -> &'a u8 {
476476
unimplemented!()
477477
}
478478
}
479479

480+
// no lint on external macro (method in impl block)
481+
external! {
482+
struct ExternalStruct;
483+
484+
impl ExternalStruct {
485+
fn needless_lifetime_method<'a>(x: &'a u8) -> &'a u8 {
486+
unimplemented!()
487+
}
488+
}
489+
}
490+
491+
// no lint on external macro (trait method)
492+
external! {
493+
trait ExternalTrait {
494+
fn needless_lifetime_trait_method<'a>(x: &'a u8) -> &'a u8;
495+
}
496+
}
497+
498+
// no lint on external macro (extra unused lifetimes in function)
499+
external! {
500+
fn extra_unused_lifetime<'a>(x: u8) {}
501+
}
502+
480503
inline! {
481504
fn f<$'a>(arg: &$'a str) -> &$'a str {
482505
arg

tests/ui/needless_lifetimes.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,13 +470,36 @@ mod in_macro {
470470
}
471471
}
472472

473-
// no lint on external macro
473+
// no lint on external macro (standalone function)
474474
external! {
475475
fn needless_lifetime<'a>(x: &'a u8) -> &'a u8 {
476476
unimplemented!()
477477
}
478478
}
479479

480+
// no lint on external macro (method in impl block)
481+
external! {
482+
struct ExternalStruct;
483+
484+
impl ExternalStruct {
485+
fn needless_lifetime_method<'a>(x: &'a u8) -> &'a u8 {
486+
unimplemented!()
487+
}
488+
}
489+
}
490+
491+
// no lint on external macro (trait method)
492+
external! {
493+
trait ExternalTrait {
494+
fn needless_lifetime_trait_method<'a>(x: &'a u8) -> &'a u8;
495+
}
496+
}
497+
498+
// no lint on external macro (extra unused lifetimes in function)
499+
external! {
500+
fn extra_unused_lifetime<'a>(x: u8) {}
501+
}
502+
480503
inline! {
481504
fn f<$'a>(arg: &$'a str) -> &$'a str {
482505
arg

0 commit comments

Comments
 (0)