Skip to content

Commit

Permalink
Auto merge of rust-lang#96833 - cjgillot:ast-lifetimes-single, r=petr…
Browse files Browse the repository at this point in the history
…ochenkov

Lint single-use lifetimes during AST resolution

This PR rewrites `single_use_lifetime` and `unused_lifetime` lints to be based on the AST.
We have more information at our disposal, so we can reduce the amount of false positives.

Remaining false positive: single-use lifetimes in argument-position impl-trait.
I'm waiting for rust-lang#96529 to be fixed to have a clean and proper solution here.

Closes rust-lang#54079
Closes rust-lang#55057
Closes rust-lang#55058
Closes rust-lang#60554
Closes rust-lang#69952

r? `@petrochenkov`
  • Loading branch information
bors committed May 20, 2022
2 parents 22ee395 + 563916d commit b5caa5a
Show file tree
Hide file tree
Showing 26 changed files with 396 additions and 493 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1976,6 +1976,8 @@ pub struct BareFnTy {
pub ext: Extern,
pub generic_params: Vec<GenericParam>,
pub decl: P<FnDecl>,
/// Span of the `fn(...) -> ...` part.
pub decl_span: Span,
}

/// The various kinds of type recognized by the compiler.
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,10 +460,11 @@ pub fn noop_visit_ty<T: MutVisitor>(ty: &mut P<Ty>, vis: &mut T) {
vis.visit_mt(mt);
}
TyKind::BareFn(bft) => {
let BareFnTy { unsafety, ext: _, generic_params, decl } = bft.deref_mut();
let BareFnTy { unsafety, ext: _, generic_params, decl, decl_span } = bft.deref_mut();
visit_unsafety(unsafety, vis);
generic_params.flat_map_in_place(|param| vis.flat_map_generic_param(param));
vis.visit_fn_decl(decl);
vis.visit_span(decl_span);
}
TyKind::Tup(tys) => visit_vec(tys, |ty| vis.visit_ty(ty)),
TyKind::Paren(ty) => vis.visit_ty(ty),
Expand Down
22 changes: 17 additions & 5 deletions compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ impl<'a> FnKind<'a> {
}
}

#[derive(Copy, Clone, Debug)]
pub enum LifetimeCtxt {
/// Appears in a reference type.
Rptr,
/// Appears as a bound on a type or another lifetime.
Bound,
/// Appears as a generic argument.
GenericArg,
}

/// Each method of the `Visitor` trait is a hook to be potentially
/// overridden. Each method's default implementation recursively visits
/// the substructure of the input via the corresponding `walk` method;
Expand Down Expand Up @@ -184,7 +194,7 @@ pub trait Visitor<'ast>: Sized {
fn visit_label(&mut self, label: &'ast Label) {
walk_label(self, label)
}
fn visit_lifetime(&mut self, lifetime: &'ast Lifetime) {
fn visit_lifetime(&mut self, lifetime: &'ast Lifetime, _: LifetimeCtxt) {
walk_lifetime(self, lifetime)
}
fn visit_mac_call(&mut self, mac: &'ast MacCall) {
Expand Down Expand Up @@ -414,7 +424,7 @@ pub fn walk_ty<'a, V: Visitor<'a>>(visitor: &mut V, typ: &'a Ty) {
TyKind::Slice(ref ty) | TyKind::Paren(ref ty) => visitor.visit_ty(ty),
TyKind::Ptr(ref mutable_type) => visitor.visit_ty(&mutable_type.ty),
TyKind::Rptr(ref opt_lifetime, ref mutable_type) => {
walk_list!(visitor, visit_lifetime, opt_lifetime);
walk_list!(visitor, visit_lifetime, opt_lifetime, LifetimeCtxt::Rptr);
visitor.visit_ty(&mutable_type.ty)
}
TyKind::Tup(ref tuple_element_types) => {
Expand Down Expand Up @@ -507,7 +517,7 @@ where
V: Visitor<'a>,
{
match generic_arg {
GenericArg::Lifetime(lt) => visitor.visit_lifetime(lt),
GenericArg::Lifetime(lt) => visitor.visit_lifetime(lt, LifetimeCtxt::GenericArg),
GenericArg::Type(ty) => visitor.visit_ty(ty),
GenericArg::Const(ct) => visitor.visit_anon_const(ct),
}
Expand Down Expand Up @@ -599,7 +609,9 @@ pub fn walk_foreign_item<'a, V: Visitor<'a>>(visitor: &mut V, item: &'a ForeignI
pub fn walk_param_bound<'a, V: Visitor<'a>>(visitor: &mut V, bound: &'a GenericBound) {
match *bound {
GenericBound::Trait(ref typ, ref modifier) => visitor.visit_poly_trait_ref(typ, modifier),
GenericBound::Outlives(ref lifetime) => visitor.visit_lifetime(lifetime),
GenericBound::Outlives(ref lifetime) => {
visitor.visit_lifetime(lifetime, LifetimeCtxt::Bound)
}
}
}

Expand Down Expand Up @@ -639,7 +651,7 @@ pub fn walk_where_predicate<'a, V: Visitor<'a>>(visitor: &mut V, predicate: &'a
WherePredicate::RegionPredicate(WhereRegionPredicate {
ref lifetime, ref bounds, ..
}) => {
visitor.visit_lifetime(lifetime);
visitor.visit_lifetime(lifetime, LifetimeCtxt::Bound);
walk_list!(visitor, visit_param_bound, bounds, BoundKind::Bound);
}
WherePredicate::EqPredicate(WhereEqPredicate { ref lhs_ty, ref rhs_ty, .. }) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
visit::walk_label(self, label);
}

fn visit_lifetime(&mut self, lifetime: &'a Lifetime) {
fn visit_lifetime(&mut self, lifetime: &'a Lifetime, _: visit::LifetimeCtxt) {
self.check_lifetime(lifetime.ident);
visit::walk_lifetime(self, lifetime);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_passes/src/node_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl<'ast> Visitor<'ast> for NodeCounter {
self.count += 1;
walk_variant(self, v)
}
fn visit_lifetime(&mut self, lifetime: &Lifetime) {
fn visit_lifetime(&mut self, lifetime: &Lifetime, _: visit::LifetimeCtxt) {
self.count += 1;
walk_lifetime(self, lifetime)
}
Expand Down
37 changes: 37 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,43 @@ pub trait LintContext: Sized {
"see issue #89122 <https://github.com/rust-lang/rust/issues/89122> for more information",
);
},
BuiltinLintDiagnostics::SingleUseLifetime {
param_span,
use_span: Some((use_span, elide)),
deletion_span,
} => {
debug!(?param_span, ?use_span, ?deletion_span);
db.span_label(param_span, "this lifetime...");
db.span_label(use_span, "...is used only here");
let msg = "elide the single-use lifetime";
let (use_span, replace_lt) = if elide {
let use_span = sess.source_map().span_extend_while(
use_span,
char::is_whitespace,
).unwrap_or(use_span);
(use_span, String::new())
} else {
(use_span, "'_".to_owned())
};
db.multipart_suggestion(
msg,
vec![(deletion_span, String::new()), (use_span, replace_lt)],
Applicability::MachineApplicable,
);
},
BuiltinLintDiagnostics::SingleUseLifetime {
param_span: _,
use_span: None,
deletion_span,
} => {
debug!(?deletion_span);
db.span_suggestion(
deletion_span,
"elide the unused lifetime",
String::new(),
Applicability::MachineApplicable,
);
},
}
// Rewrap `db`, and pass control to the user.
decorate(LintDiagnosticBuilder::new(db));
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_lint/src/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>

fn visit_generic_param(&mut self, param: &'a ast::GenericParam) {
run_early_pass!(self, check_generic_param, param);
self.check_id(param.id);
ast_visit::walk_generic_param(self, param);
}

Expand Down Expand Up @@ -272,7 +273,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
});
}

fn visit_lifetime(&mut self, lt: &'a ast::Lifetime) {
fn visit_lifetime(&mut self, lt: &'a ast::Lifetime, _: ast_visit::LifetimeCtxt) {
run_early_pass!(self, check_lifetime, lt);
self.check_id(lt.id);
}
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_middle::lint::{
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{RegisteredTools, TyCtxt};
use rustc_session::lint::{
builtin::{self, FORBIDDEN_LINT_GROUPS, UNFULFILLED_LINT_EXPECTATIONS},
builtin::{self, FORBIDDEN_LINT_GROUPS, SINGLE_USE_LIFETIMES, UNFULFILLED_LINT_EXPECTATIONS},
Level, Lint, LintExpectationId, LintId,
};
use rustc_session::parse::{add_feature_diagnostics, feature_err};
Expand Down Expand Up @@ -259,6 +259,14 @@ impl<'s> LintLevelsBuilder<'s> {
let sess = self.sess;
let bad_attr = |span| struct_span_err!(sess, span, E0452, "malformed lint attribute input");
for (attr_index, attr) in attrs.iter().enumerate() {
if attr.has_name(sym::automatically_derived) {
self.current_specs_mut().insert(
LintId::of(SINGLE_USE_LIFETIMES),
(Level::Allow, LintLevelSource::Default),
);
continue;
}

let level = match Level::from_attr(attr) {
None => continue,
Some(Level::Expect(unstable_id)) if let Some(hir_id) = source_hir_id => {
Expand Down
16 changes: 15 additions & 1 deletion compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,11 @@ pub enum BuiltinLintDiagnostics {
DeprecatedMacro(Option<Symbol>, Span),
MissingAbi(Span, Abi),
UnusedDocComment(Span),
UnusedBuiltinAttribute { attr_name: Symbol, macro_name: String, invoc_span: Span },
UnusedBuiltinAttribute {
attr_name: Symbol,
macro_name: String,
invoc_span: Span,
},
PatternsInFnsWithoutBody(Span, Ident),
LegacyDeriveHelpers(Span),
ProcMacroBackCompat(String),
Expand All @@ -435,6 +439,16 @@ pub enum BuiltinLintDiagnostics {
UnicodeTextFlow(Span, String),
UnexpectedCfg((Symbol, Span), Option<(Symbol, Span)>),
DeprecatedWhereclauseLocation(Span, String),
SingleUseLifetime {
/// Span of the parameter which declares this lifetime.
param_span: Span,
/// Span of the code that should be removed when eliding this lifetime.
/// This span should include leading or trailing comma.
deletion_span: Span,
/// Span of the single use, or None if the lifetime is never used.
/// If true, the lifetime will be fully elided.
use_span: Option<(Span, bool)>,
},
}

/// Lints that are buffered up early on in the `Session` before the
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ impl<'a> Parser<'a> {
kind: rustc_ast::VisibilityKind::Inherited,
tokens: None,
};
let span_start = self.token.span;
let ast::FnHeader { ext, unsafety, constness, asyncness } =
self.parse_fn_front_matter(&inherited_vis)?;
let decl = self.parse_fn_decl(|_| false, AllowPlus::No, recover_return_sign)?;
Expand All @@ -531,7 +532,8 @@ impl<'a> Parser<'a> {
if let ast::Async::Yes { span, .. } = asyncness {
self.error_fn_ptr_bad_qualifier(whole_span, span, "async");
}
Ok(TyKind::BareFn(P(BareFnTy { ext, unsafety, generic_params: params, decl })))
let decl_span = span_start.to(self.token.span);
Ok(TyKind::BareFn(P(BareFnTy { ext, unsafety, generic_params: params, decl, decl_span })))
}

/// Emit an error for the given bad function pointer qualifier.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/hir_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ impl<'v> ast_visit::Visitor<'v> for StatCollector<'v> {
ast_visit::walk_variant(self, v)
}

fn visit_lifetime(&mut self, lifetime: &'v ast::Lifetime) {
fn visit_lifetime(&mut self, lifetime: &'v ast::Lifetime, _: ast_visit::LifetimeCtxt) {
self.record("Lifetime", Id::None, lifetime);
ast_visit::walk_lifetime(self, lifetime)
}
Expand Down
Loading

0 comments on commit b5caa5a

Please sign in to comment.