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

Move elided_named_lifetimes into a separate pass #130150

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions compiler/rustc_hir/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -829,12 +829,7 @@ pub enum LifetimeRes {
/// late resolution. Those lifetimes will be inferred by typechecking.
Infer,
/// `'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,
},
Static,
/// Resolution failure.
Error,
/// HACK: This is used to recover the NodeId of an elided lifetime.
Expand Down
16 changes: 2 additions & 14 deletions compiler/rustc_lint/src/context/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ use rustc_errors::{
elided_lifetime_in_path_suggestion, Applicability, Diag, DiagArgValue, LintDiagnostic,
};
use rustc_middle::middle::stability;
use rustc_session::lint::{BuiltinLintDiag, ElidedLifetimeResolution};
use rustc_session::lint::BuiltinLintDiag;
use rustc_session::Session;
use rustc_span::symbol::kw;
use rustc_span::BytePos;
use tracing::debug;

use crate::lints::{self, ElidedNamedLifetime};
use crate::lints::{self};

mod check_cfg;

Expand Down Expand Up @@ -446,16 +445,5 @@ 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::ElidedNamedLifetimes { elided: (span, kind), resolution } => {
match resolution {
ElidedLifetimeResolution::Static => {
ElidedNamedLifetime { span, kind, name: kw::StaticLifetime, declaration: None }
}
ElidedLifetimeResolution::Param(name, declaration) => {
ElidedNamedLifetime { span, kind, name, declaration: Some(declaration) }
}
}
.decorate_lint(diag)
}
}
}
116 changes: 116 additions & 0 deletions compiler/rustc_lint/src/elided.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
use rustc_hir::{
ImplItem, ImplItemKind, Item, ItemKind, Lifetime, LifetimeName, TraitItem, TraitItemKind,
};
use rustc_middle::ty::layout::HasTyCtxt;
use rustc_session::{declare_lint, impl_lint_pass};
use rustc_span::symbol::kw;

use crate::lints::{ElidedNamedLifetime, ElidedNamedLifetimeSuggestion};
use crate::{LateContext, LateLintPass, LintContext};

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"
}

#[derive(Clone, Debug, Default)]
pub(crate) struct ElidedNamedLifetimes {
allow_static: bool,
}

impl_lint_pass!(ElidedNamedLifetimes => [ELIDED_NAMED_LIFETIMES]);

impl<'tcx> LateLintPass<'tcx> for ElidedNamedLifetimes {
fn check_trait_item(&mut self, _: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) {
if let TraitItemKind::Const(..) = item.kind {
self.allow_static = true;
}
}
fn check_trait_item_post(&mut self, _: &LateContext<'tcx>, _: &'tcx TraitItem<'tcx>) {
self.allow_static = false;
}

fn check_impl_item(&mut self, _: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) {
if let ImplItemKind::Const(..) = item.kind {
self.allow_static = true;
}
}
fn check_impl_item_post(&mut self, _: &LateContext<'tcx>, _: &'tcx ImplItem<'tcx>) {
self.allow_static = false;
}

fn check_item(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if let ItemKind::Const(..) | ItemKind::Static(..) = item.kind {
self.allow_static = true;
}
}
fn check_item_post(&mut self, _: &LateContext<'tcx>, _: &'tcx Item<'tcx>) {
self.allow_static = false;
}
Comment on lines +51 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced this handles cases like:

const FOO: ConstGeneric<{
    const BAR: usize = 5;
    BAR
}, &u8> = /* */;

You may need an actual stack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just storing the id of the outermost item would be enough.

However, I am reluctant to move any further with this PR as I suspect some lifetimes, which are present in AST, are not present in HIR and thus are unlintable with this approach.


fn check_lifetime(&mut self, cx: &LateContext<'tcx>, lifetime: &'tcx Lifetime) {
// `.is_elided()` should probably be called `.resolves_to_elided()`,
// and `.is_anonymous()` is actually the thing that we need here.
if !lifetime.is_anonymous() {
return;
}
let (name, declaration) = match lifetime.res {
LifetimeName::Param(param) => {
let name = cx.tcx().item_name(param.into());
if name == kw::UnderscoreLifetime {
return;
}
let span = cx.tcx().def_span(param);
(name, Some(span))
}
LifetimeName::Static => {
if self.allow_static {
return;
}
(kw::StaticLifetime, None)
}
LifetimeName::ImplicitObjectLifetimeDefault
| LifetimeName::Error
| LifetimeName::Infer => return,
};
cx.emit_lint(
ELIDED_NAMED_LIFETIMES,
ElidedNamedLifetime {
span: lifetime.ident.span,
sugg: {
let (span, code) = lifetime.suggestion(name.as_str());
ElidedNamedLifetimeSuggestion { span, code }
},
name,
declaration,
},
)
}
}
2 changes: 2 additions & 0 deletions compiler/rustc_lint/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
cx.with_param_env(trait_item.owner_id, |cx| {
lint_callback!(cx, check_trait_item, trait_item);
hir_visit::walk_trait_item(cx, trait_item);
lint_callback!(cx, check_trait_item_post, trait_item);
});
});
self.context.generics = generics;
Expand All @@ -305,6 +306,7 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
}

fn visit_lifetime(&mut self, lt: &'tcx hir::Lifetime) {
lint_callback!(self, check_lifetime, lt);
hir_visit::walk_lifetime(self, lt);
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ mod context;
mod deref_into_dyn_supertrait;
mod drop_forget_useless;
mod early;
mod elided;
mod enum_intrinsics_non_enums;
mod errors;
mod expect;
Expand Down Expand Up @@ -91,6 +92,7 @@ use async_fn_in_trait::AsyncFnInTrait;
use builtin::*;
use deref_into_dyn_supertrait::*;
use drop_forget_useless::*;
use elided::ElidedNamedLifetimes;
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
use for_loops_over_fallibles::*;
use hidden_unicode_codepoints::*;
Expand Down Expand Up @@ -243,6 +245,7 @@ late_lint_methods!(
NonLocalDefinitions: NonLocalDefinitions::default(),
ImplTraitOvercaptures: ImplTraitOvercaptures,
TailExprDropOrder: TailExprDropOrder,
ElidedNamedLifetimes: ElidedNamedLifetimes::default(),
]
]
);
Expand Down
45 changes: 16 additions & 29 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ use rustc_errors::{
Applicability, Diag, DiagArgValue, DiagMessage, DiagStyledString, ElidedLifetimeInPathSubdiag,
EmissionGuarantee, LintDiagnostic, MultiSpan, SubdiagMessageOp, Subdiagnostic, SuggestionStyle,
};
use rustc_hir as hir;
use rustc_hir::def::Namespace;
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, MissingLifetimeKind};
use rustc_macros::{LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::inhabitedness::InhabitedPredicate;
use rustc_middle::ty::{Clause, PolyExistentialTraitRef, Ty, TyCtxt};
Expand Down Expand Up @@ -2624,16 +2624,23 @@ pub(crate) struct ElidedLifetimesInPaths {
pub subdiag: ElidedLifetimeInPathSubdiag,
}

#[allow(unused)]
pub(crate) struct ElidedNamedLifetime {
pub span: Span,
pub kind: MissingLifetimeKind,
pub name: Symbol,
pub declaration: Option<Span>,
pub sugg: ElidedNamedLifetimeSuggestion,
}

pub(crate) struct ElidedNamedLifetimeSuggestion {
pub span: Span,
pub code: String,
}

impl<G: EmissionGuarantee> LintDiagnostic<'_, G> for ElidedNamedLifetime {
fn decorate_lint(self, diag: &mut rustc_errors::Diag<'_, G>) {
let Self { span, kind, name, declaration } = self;
let Self { span, name, declaration, sugg } = self;
diag.span(span);
diag.primary_message(fluent::lint_elided_named_lifetime);
diag.arg("name", name);
diag.span_label(span, fluent::lint_label_elided);
Expand All @@ -2647,32 +2654,12 @@ impl<G: EmissionGuarantee> LintDiagnostic<'_, G> for ElidedNamedLifetime {
if name != rustc_span::symbol::kw::StaticLifetime {
return;
}
match kind {
MissingLifetimeKind::Underscore => diag.span_suggestion_verbose(
span,
fluent::lint_suggestion,
format!("{name}"),
Applicability::MachineApplicable,
),
MissingLifetimeKind::Ampersand => diag.span_suggestion_verbose(
span.shrink_to_hi(),
fluent::lint_suggestion,
format!("{name} "),
Applicability::MachineApplicable,
),
MissingLifetimeKind::Comma => diag.span_suggestion_verbose(
span.shrink_to_hi(),
fluent::lint_suggestion,
format!("{name}, "),
Applicability::MachineApplicable,
),
MissingLifetimeKind::Brackets => diag.span_suggestion_verbose(
span.shrink_to_hi(),
fluent::lint_suggestion,
format!("<{name}>"),
Applicability::MachineApplicable,
),
};
diag.span_suggestion_verbose(
sugg.span,
fluent::lint_suggestion,
sugg.code,
Applicability::MachineApplicable,
);
}
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_lint/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ macro_rules! late_lint_methods {
d: rustc_span::Span,
e: rustc_span::def_id::LocalDefId);
fn check_trait_item(a: &'tcx rustc_hir::TraitItem<'tcx>);
fn check_trait_item_post(a: &'tcx rustc_hir::TraitItem<'tcx>);
fn check_impl_item(a: &'tcx rustc_hir::ImplItem<'tcx>);
fn check_impl_item_post(a: &'tcx rustc_hir::ImplItem<'tcx>);
fn check_struct_def(a: &'tcx rustc_hir::VariantData<'tcx>);
Expand All @@ -43,6 +44,7 @@ macro_rules! late_lint_methods {
fn check_attribute(a: &'tcx rustc_ast::Attribute);
fn check_attributes(a: &'tcx [rustc_ast::Attribute]);
fn check_attributes_post(a: &'tcx [rustc_ast::Attribute]);
fn check_lifetime(lt: &'tcx rustc_hir::Lifetime);
]);
)
}
Expand Down
33 changes: 0 additions & 33 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ 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 @@ -1863,38 +1862,6 @@ 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
12 changes: 1 addition & 11 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_data_structures::stable_hasher::{
};
use rustc_error_messages::{DiagMessage, MultiSpan};
use rustc_hir::def::Namespace;
use rustc_hir::{HashStableContext, HirId, MissingLifetimeKind};
use rustc_hir::{HashStableContext, HirId};
use rustc_macros::{Decodable, Encodable, HashStable_Generic};
use rustc_span::edition::Edition;
use rustc_span::symbol::{Ident, MacroRulesNormalizedIdent};
Expand Down Expand Up @@ -556,12 +556,6 @@ pub enum DeprecatedSinceKind {
InVersion(String),
}

#[derive(Debug)]
pub enum ElidedLifetimeResolution {
Static,
Param(Symbol, Span),
}

// This could be a closure, but then implementing derive trait
// becomes hacky (and it gets allocated).
#[derive(Debug)]
Expand All @@ -574,10 +568,6 @@ pub enum BuiltinLintDiag {
},
MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
ElidedLifetimesInPaths(usize, Span, bool, Span),
ElidedNamedLifetimes {
elided: (Span, MissingLifetimeKind),
resolution: ElidedLifetimeResolution,
},
UnknownCrateTypes {
span: Span,
candidate: Option<Symbol>,
Expand Down
Loading
Loading