Skip to content

Commit

Permalink
Implement elided_named_lifetimes lint
Browse files Browse the repository at this point in the history
  • Loading branch information
GrigorenkoPV committed Aug 31, 2024
1 parent 0d63418 commit 5d04472
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 20 deletions.
6 changes: 3 additions & 3 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

(hir::ParamName::Fresh, hir::LifetimeParamKind::Elided(kind))
}
LifetimeRes::Static | LifetimeRes::Error => return None,
LifetimeRes::Static { .. } | LifetimeRes::Error => return None,
res => panic!(
"Unexpected lifetime resolution {:?} for {:?} at {:?}",
res, ident, ident.span
Expand Down Expand Up @@ -1656,7 +1656,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}

// Opaques do not capture `'static`
LifetimeRes::Static | LifetimeRes::Error => {
LifetimeRes::Static { .. } | LifetimeRes::Error => {
continue;
}

Expand Down Expand Up @@ -2069,7 +2069,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
hir::LifetimeName::Param(param)
}
LifetimeRes::Infer => hir::LifetimeName::Infer,
LifetimeRes::Static => hir::LifetimeName::Static,
LifetimeRes::Static { .. } => hir::LifetimeName::Static,
LifetimeRes::Error => hir::LifetimeName::Error,
res => panic!(
"Unexpected lifetime resolution {:?} for {:?} at {:?}",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/lifetime_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl<'ast> LifetimeCollectVisitor<'ast> {
self.collected_lifetimes.insert(lifetime);
}
}
LifetimeRes::Static | LifetimeRes::Error => {
LifetimeRes::Static { .. } | LifetimeRes::Error => {
self.collected_lifetimes.insert(lifetime);
}
LifetimeRes::Infer => {}
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_hir/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -863,8 +863,13 @@ pub enum LifetimeRes {
/// This variant is used for anonymous lifetimes that we did not resolve during
/// late resolution. Those lifetimes will be inferred by typechecking.
Infer,
/// Explicit `'static` lifetime.
Static,
/// `'static` lifetime.
Static {
/// We do not want to emit `elided_named_lifetimes`
/// when we are inside of a const item or a static,
/// because it would get too annoying.
suppress_elision_warning: bool,
},
/// Resolution failure.
Error,
/// HACK: This is used to recover the NodeId of an elided lifetime.
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ lint_duplicate_macro_attribute =
lint_duplicate_matcher_binding = duplicate matcher binding
lint_elided_named_lifetime = elided lifetime has a name
.label_elided = this elided lifetime gets resolved as `{$name}`
.label_named = lifetime `{$name}` declared here
lint_enum_intrinsics_mem_discriminant =
the return value of `mem::discriminant` is unspecified when called with a non-enum type
.note = the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `{$ty_param}`, which is not an enum
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_lint/src/context/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_errors::{
use rustc_middle::middle::stability;
use rustc_session::lint::BuiltinLintDiag;
use rustc_session::Session;
use rustc_span::symbol::kw;
use rustc_span::BytePos;
use tracing::debug;

Expand Down Expand Up @@ -441,5 +442,17 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: &
BuiltinLintDiag::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by } => {
lints::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by }.decorate_lint(diag)
}
BuiltinLintDiag::ElidedIsStatic { elided } => {
lints::ElidedNamedLifetime { elided, name: kw::StaticLifetime, named_declaration: None }
.decorate_lint(diag)
}
BuiltinLintDiag::ElidedIsParam { elided, param: (param_name, param_span) } => {
lints::ElidedNamedLifetime {
elided,
name: param_name,
named_declaration: Some(param_span),
}
.decorate_lint(diag)
}
}
}
10 changes: 10 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2611,6 +2611,16 @@ pub(crate) struct ElidedLifetimesInPaths {
pub subdiag: ElidedLifetimeInPathSubdiag,
}

#[derive(LintDiagnostic)]
#[diag(lint_elided_named_lifetime)]
pub(crate) struct ElidedNamedLifetime {
#[label(lint_label_elided)]
pub elided: Span,
pub name: Symbol,
#[label(lint_label_named)]
pub named_declaration: Option<Span>,
}

#[derive(LintDiagnostic)]
#[diag(lint_invalid_crate_type_value)]
pub(crate) struct UnknownCrateTypes {
Expand Down
33 changes: 33 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ declare_lint_pass! {
DUPLICATE_MACRO_ATTRIBUTES,
ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
ELIDED_LIFETIMES_IN_PATHS,
ELIDED_NAMED_LIFETIMES,
EXPLICIT_BUILTIN_CFGS_IN_FLAGS,
EXPORTED_PRIVATE_DEPENDENCIES,
FFI_UNWIND_CALLS,
Expand Down Expand Up @@ -1862,6 +1863,38 @@ declare_lint! {
"hidden lifetime parameters in types are deprecated"
}

declare_lint! {
/// The `elided_named_lifetimes` lint detects when an elided
/// lifetime ends up being a named lifetime, such as `'static`
/// or some lifetime parameter `'a`.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(elided_named_lifetimes)]
/// struct Foo;
/// impl Foo {
/// pub fn get_mut(&'static self, x: &mut u8) -> &mut u8 {
/// unsafe { &mut *(x as *mut _) }
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Lifetime elision is quite useful, because it frees you from having
/// to give each lifetime its own name, but sometimes it can produce
/// somewhat surprising resolutions. In safe code, it is mostly okay,
/// because the borrow checker prevents any unsoundness, so the worst
/// case scenario is you get a confusing error message in some other place.
/// But with `unsafe` code, such unexpected resolutions may lead to unsound code.
pub ELIDED_NAMED_LIFETIMES,
Warn,
"detects when an elided lifetime gets resolved to be `'static` or some named parameter"
}

declare_lint! {
/// The `bare_trait_objects` lint suggests using `dyn Trait` for trait
/// objects.
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,13 @@ pub enum BuiltinLintDiag {
},
MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
ElidedLifetimesInPaths(usize, Span, bool, Span),
ElidedIsStatic {
elided: Span,
},
ElidedIsParam {
elided: Span,
param: (Symbol, Span),
},
UnknownCrateTypes {
span: Span,
candidate: Option<Symbol>,
Expand Down
79 changes: 67 additions & 12 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1557,14 +1557,14 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
if ident.name == kw::StaticLifetime {
self.record_lifetime_res(
lifetime.id,
LifetimeRes::Static,
LifetimeRes::Static { suppress_elision_warning: false },
LifetimeElisionCandidate::Named,
);
return;
}

if ident.name == kw::UnderscoreLifetime {
return self.resolve_anonymous_lifetime(lifetime, false);
return self.resolve_anonymous_lifetime(lifetime, lifetime.id, false);
}

let mut lifetime_rib_iter = self.lifetime_ribs.iter().rev();
Expand Down Expand Up @@ -1667,13 +1667,23 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
}

#[instrument(level = "debug", skip(self))]
fn resolve_anonymous_lifetime(&mut self, lifetime: &Lifetime, elided: bool) {
fn resolve_anonymous_lifetime(
&mut self,
lifetime: &Lifetime,
id_for_lint: NodeId,
elided: bool,
) {
debug_assert_eq!(lifetime.ident.name, kw::UnderscoreLifetime);

let kind =
if elided { MissingLifetimeKind::Ampersand } else { MissingLifetimeKind::Underscore };
let missing_lifetime =
MissingLifetime { id: lifetime.id, span: lifetime.ident.span, kind, count: 1 };
let missing_lifetime = MissingLifetime {
id: lifetime.id,
span: lifetime.ident.span,
kind,
count: 1,
id_for_lint,
};
let elision_candidate = LifetimeElisionCandidate::Missing(missing_lifetime);
for (i, rib) in self.lifetime_ribs.iter().enumerate().rev() {
debug!(?rib.kind);
Expand All @@ -1697,7 +1707,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
if lifetimes_in_scope.is_empty() {
self.record_lifetime_res(
lifetime.id,
LifetimeRes::Static,
// We are inside a const item, so do not warn.
LifetimeRes::Static { suppress_elision_warning: true },
elision_candidate,
);
return;
Expand Down Expand Up @@ -1800,7 +1811,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
LifetimeRes::ElidedAnchor { start: id, end: NodeId::from_u32(id.as_u32() + 1) },
LifetimeElisionCandidate::Ignore,
);
self.resolve_anonymous_lifetime(&lt, true);
self.resolve_anonymous_lifetime(&lt, anchor_id, true);
}

#[instrument(level = "debug", skip(self))]
Expand Down Expand Up @@ -1916,6 +1927,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
};
let missing_lifetime = MissingLifetime {
id: node_ids.start,
id_for_lint: segment_id,
span: elided_lifetime_span,
kind,
count: expected_lifetimes,
Expand Down Expand Up @@ -2039,8 +2051,44 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
if let Some(prev_res) = self.r.lifetimes_res_map.insert(id, res) {
panic!("lifetime {id:?} resolved multiple times ({prev_res:?} before, {res:?} now)")
}

match candidate {
LifetimeElisionCandidate::Missing(missing @ MissingLifetime { .. }) => {
debug_assert_eq!(id, missing.id);
match res {
LifetimeRes::Static { suppress_elision_warning } => {
if !suppress_elision_warning {
self.r.lint_buffer.buffer_lint(
lint::builtin::ELIDED_NAMED_LIFETIMES,
missing.id_for_lint,
missing.span,
BuiltinLintDiag::ElidedIsStatic { elided: missing.span },
);
}
}
LifetimeRes::Param { param, binder: _ } => {
let tcx = self.r.tcx();
self.r.lint_buffer.buffer_lint(
lint::builtin::ELIDED_NAMED_LIFETIMES,
missing.id_for_lint,
missing.span,
BuiltinLintDiag::ElidedIsParam {
elided: missing.span,
param: (tcx.item_name(param.into()), tcx.source_span(param)),
},
);
}
LifetimeRes::Fresh { .. }
| LifetimeRes::Infer
| LifetimeRes::Error
| LifetimeRes::ElidedAnchor { .. } => {}
}
}
LifetimeElisionCandidate::Ignore | LifetimeElisionCandidate::Named => {}
}

match res {
LifetimeRes::Param { .. } | LifetimeRes::Fresh { .. } | LifetimeRes::Static => {
LifetimeRes::Param { .. } | LifetimeRes::Fresh { .. } | LifetimeRes::Static { .. } => {
if let Some(ref mut candidates) = self.lifetime_elision_candidates {
candidates.push((res, candidate));
}
Expand Down Expand Up @@ -2558,9 +2606,14 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {

ItemKind::Static(box ast::StaticItem { ref ty, ref expr, .. }) => {
self.with_static_rib(def_kind, |this| {
this.with_lifetime_rib(LifetimeRibKind::Elided(LifetimeRes::Static), |this| {
this.visit_ty(ty);
});
this.with_lifetime_rib(
LifetimeRibKind::Elided(LifetimeRes::Static {
suppress_elision_warning: true,
}),
|this| {
this.visit_ty(ty);
},
);
if let Some(expr) = expr {
// We already forbid generic params because of the above item rib,
// so it doesn't matter whether this is a trivial constant.
Expand Down Expand Up @@ -2589,7 +2642,9 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
this.visit_generics(generics);

this.with_lifetime_rib(
LifetimeRibKind::Elided(LifetimeRes::Static),
LifetimeRibKind::Elided(LifetimeRes::Static {
suppress_elision_warning: true,
}),
|this| this.visit_ty(ty),
);

Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,13 @@ fn import_candidate_to_enum_paths(suggestion: &ImportSuggestion) -> (String, Str
pub(super) struct MissingLifetime {
/// Used to overwrite the resolution with the suggestion, to avoid cascading errors.
pub id: NodeId,
/// As we cannot yet emit lints in this crate and have to buffer them instead,
/// we need to associate each lint with some `NodeId`,
/// however for some `MissingLifetime`s their `NodeId`s are "fake",
/// in a sense that they are temporary and not get preserved down the line,
/// which means that the lints for those nodes will not get emitted.
/// To combat this, we can try to use some other `NodeId`s as a fallback option.
pub id_for_lint: NodeId,
/// Where to suggest adding the lifetime.
pub span: Span,
/// How the lifetime was introduced, to have the correct space and comma.
Expand Down Expand Up @@ -3028,7 +3035,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
maybe_static = true;
in_scope_lifetimes = vec![(
Ident::with_dummy_span(kw::StaticLifetime),
(DUMMY_NODE_ID, LifetimeRes::Static),
(DUMMY_NODE_ID, LifetimeRes::Static { suppress_elision_warning: false }),
)];
}
} else if elided_len == 0 {
Expand All @@ -3040,7 +3047,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
maybe_static = true;
in_scope_lifetimes = vec![(
Ident::with_dummy_span(kw::StaticLifetime),
(DUMMY_NODE_ID, LifetimeRes::Static),
(DUMMY_NODE_ID, LifetimeRes::Static { suppress_elision_warning: false }),
)];
}
} else if num_params == 1 {
Expand Down

0 comments on commit 5d04472

Please sign in to comment.