Skip to content

Commit

Permalink
Split elided_lifetime_in_paths into tied and untied
Browse files Browse the repository at this point in the history
Types that contain a reference can be confusing when lifetime elision
occurs:

```rust
// Confusing
fn foo(_: &u8) -> Bar { todo!() }

// Less confusing
fn foo(_: &u8) -> Bar<'_> { todo!() }
```

However, the previous lint did not distinguish when these types were
not "tying" lifetimes across the function inputs / outputs:

```rust
// Maybe a little confusing
fn foo(_: Bar) {}

// More explicit but noisier with less obvious value
fn foo(_: Bar<'_>) {}
```

We now report different lints for each case, hopefully paving the way
to marking the first case (when lifetimes are tied together) as
warn-by-default.

Additionally, when multiple errors occur in the same function during
the tied case, they are coalesced into one error. There is also some
help text pointing out where the lifetime source is.
  • Loading branch information
shepmaster committed Feb 12, 2025
1 parent efba226 commit 50802c5
Show file tree
Hide file tree
Showing 17 changed files with 751 additions and 40 deletions.
16 changes: 12 additions & 4 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1937,16 +1937,24 @@ pub fn elided_lifetime_in_path_suggestion(
insertion_span: Span,
) -> ElidedLifetimeInPathSubdiag {
let expected = ExpectedLifetimeParameter { span: path_span, count: n };
let indicate = indicate_anonymous_lifetime(source_map, n, incl_angl_brckt, insertion_span);
ElidedLifetimeInPathSubdiag { expected, indicate }
}

pub fn indicate_anonymous_lifetime(
source_map: &SourceMap,
n: usize,
incl_angl_brckt: bool,
insertion_span: Span,
) -> Option<IndicateAnonymousLifetime> {
// Do not try to suggest anything if generated by a proc-macro.
let indicate = source_map.is_span_accessible(insertion_span).then(|| {
source_map.is_span_accessible(insertion_span).then(|| {
let anon_lts = vec!["'_"; n].join(", ");
let suggestion =
if incl_angl_brckt { format!("<{anon_lts}>") } else { format!("{anon_lts}, ") };

IndicateAnonymousLifetime { span: insertion_span.shrink_to_hi(), count: n, suggestion }
});

ElidedLifetimeInPathSubdiag { expected, indicate }
})
}

pub fn report_ambiguity_error<'a, G: EmissionGuarantee>(
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,8 @@ lint_hidden_glob_reexport = private item shadows public glob re-export
lint_hidden_lifetime_parameters = hidden lifetime parameters in types are deprecated
lint_hidden_lifetime_parameters_tied_source = the lifetime comes from here
lint_hidden_unicode_codepoints = unicode codepoint changing visible direction of text present in {$label}
.label = this {$label} contains {$count ->
[one] an invisible
Expand Down
24 changes: 23 additions & 1 deletion compiler/rustc_lint/src/early/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use std::borrow::Cow;

use rustc_ast::util::unicode::TEXT_FLOW_CONTROL_CHARS;
use rustc_errors::{
Applicability, Diag, DiagArgValue, LintDiagnostic, elided_lifetime_in_path_suggestion,
Applicability, Diag, DiagArgValue, ExpectedLifetimeParameter, LintDiagnostic,
elided_lifetime_in_path_suggestion, indicate_anonymous_lifetime,
};
use rustc_middle::middle::stability;
use rustc_middle::ty::TyCtxt;
Expand Down Expand Up @@ -88,6 +89,27 @@ pub(super) fn decorate_lint(
}
.decorate_lint(diag);
}

BuiltinLintDiag::ElidedLifetimesInPathsTied { elided_lifetime_source, suggestions } => {
let elided_lifetime_source =
elided_lifetime_source.map(|span| lints::ElidedLifetimesInPathsTiedSource { span });

let expected = suggestions
.iter()
.map(|&(span, count, _)| ExpectedLifetimeParameter { span, count })
.collect();

let suggestions = suggestions
.into_iter()
.flat_map(|(span, n, incl_angl_brckt)| {
indicate_anonymous_lifetime(sess.source_map(), n, incl_angl_brckt, span)
})
.collect();

lints::ElidedLifetimesInPathsTied { expected, suggestions, elided_lifetime_source }
.decorate_lint(diag)
}

BuiltinLintDiag::UnknownCrateTypes { span, candidate } => {
let sugg = candidate.map(|candidate| lints::UnknownCrateTypesSub { span, candidate });
lints::UnknownCrateTypes { sugg }.decorate_lint(diag);
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ fn register_builtins(store: &mut LintStore) {
BARE_TRAIT_OBJECTS,
UNUSED_EXTERN_CRATES,
ELLIPSIS_INCLUSIVE_RANGE_PATTERNS,
ELIDED_LIFETIMES_IN_PATHS,
ELIDED_LIFETIMES_IN_PATHS_TIED,
ELIDED_LIFETIMES_IN_PATHS_UNTIED,
EXPLICIT_OUTLIVES_REQUIREMENTS,
// FIXME(#52665, #47816) not always applicable and not all
// macros are ready for this yet.
Expand All @@ -337,9 +338,14 @@ fn register_builtins(store: &mut LintStore) {

add_lint_group!("deprecated_safe", DEPRECATED_SAFE_2024);

add_lint_group!(
"elided_lifetimes_in_paths",
ELIDED_LIFETIMES_IN_PATHS_TIED,
ELIDED_LIFETIMES_IN_PATHS_UNTIED,
);

// Register renamed and removed lints.
store.register_renamed("single_use_lifetime", "single_use_lifetimes");
store.register_renamed("elided_lifetime_in_path", "elided_lifetimes_in_paths");
store.register_renamed("bare_trait_object", "bare_trait_objects");
store.register_renamed("unstable_name_collision", "unstable_name_collisions");
store.register_renamed("unused_doc_comment", "unused_doc_comments");
Expand All @@ -354,6 +360,9 @@ fn register_builtins(store: &mut LintStore) {
store.register_renamed("static_mut_ref", "static_mut_refs");
store.register_renamed("temporary_cstring_as_ptr", "dangling_pointers_from_temporaries");

// Register renamed lint groups
store.register_renamed_group("elided_lifetime_in_path", "elided_lifetimes_in_paths");

// These were moved to tool lints, but rustc still sees them when compiling normally, before
// tool lints are registered, so `check_tool_name_for_backwards_compat` doesn't work. Use
// `register_removed` explicitly.
Expand Down
23 changes: 22 additions & 1 deletion compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use rustc_abi::ExternAbi;
use rustc_errors::codes::*;
use rustc_errors::{
Applicability, Diag, DiagArgValue, DiagMessage, DiagStyledString, ElidedLifetimeInPathSubdiag,
EmissionGuarantee, LintDiagnostic, MultiSpan, SubdiagMessageOp, Subdiagnostic, SuggestionStyle,
EmissionGuarantee, ExpectedLifetimeParameter, IndicateAnonymousLifetime, LintDiagnostic,
MultiSpan, SubdiagMessageOp, Subdiagnostic, SuggestionStyle,
};
use rustc_hir::def::Namespace;
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -2728,6 +2729,26 @@ impl<G: EmissionGuarantee> LintDiagnostic<'_, G> for ElidedNamedLifetime {
}
}

#[derive(LintDiagnostic)]
#[diag(lint_hidden_lifetime_parameters)] // deliberately the same translation
pub(crate) struct ElidedLifetimesInPathsTied {
#[subdiagnostic]
pub expected: Vec<ExpectedLifetimeParameter>,

#[subdiagnostic]
pub suggestions: Vec<IndicateAnonymousLifetime>,

#[subdiagnostic]
pub elided_lifetime_source: Option<ElidedLifetimesInPathsTiedSource>,
}

#[derive(Subdiagnostic)]
#[label(lint_hidden_lifetime_parameters_tied_source)]
pub(crate) struct ElidedLifetimesInPathsTiedSource {
#[primary_span]
pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag(lint_invalid_crate_type_value)]
pub(crate) struct UnknownCrateTypes {
Expand Down
54 changes: 48 additions & 6 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ declare_lint_pass! {
DEPRECATED_WHERE_CLAUSE_LOCATION,
DUPLICATE_MACRO_ATTRIBUTES,
ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
ELIDED_LIFETIMES_IN_PATHS,
ELIDED_LIFETIMES_IN_PATHS_TIED,
ELIDED_LIFETIMES_IN_PATHS_UNTIED,
ELIDED_NAMED_LIFETIMES,
EXPLICIT_BUILTIN_CFGS_IN_FLAGS,
EXPORTED_PRIVATE_DEPENDENCIES,
Expand Down Expand Up @@ -1829,19 +1830,21 @@ declare_lint! {
}

declare_lint! {
/// The `elided_lifetimes_in_paths` lint detects the use of hidden
/// lifetime parameters.
/// The `elided_lifetimes_in_paths_tied` lint detects the use of
/// hidden lifetime parameters when those lifetime parameters tie
/// an input lifetime parameter to an output lifetime parameter.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(elided_lifetimes_in_paths)]
/// #![deny(elided_lifetimes_in_paths_tied)]
/// #![deny(warnings)]
/// struct Foo<'a> {
/// x: &'a u32
/// }
///
/// fn foo(x: &Foo) {
/// fn foo(x: Foo) -> &u32 {
/// &x.0
/// }
/// ```
///
Expand All @@ -1858,11 +1861,50 @@ declare_lint! {
/// may require a significant transition for old code.
///
/// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
pub ELIDED_LIFETIMES_IN_PATHS,
pub ELIDED_LIFETIMES_IN_PATHS_TIED,
Allow,
"hidden lifetime parameters in types are deprecated"
}

declare_lint! {
/// The `elided_lifetimes_in_paths_untied` lint detects the use of
/// hidden lifetime parameters when those lifetime parameters do
/// not tie an input lifetime parameter to an output lifetime
/// parameter.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(elided_lifetimes_in_paths_untied)]
/// #![deny(warnings)]
/// struct Foo<'a> {
/// x: &'a u32
/// }
///
/// fn foo(x: Foo) -> u32 {
/// x.0
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Elided lifetime parameters can make it difficult to see at a glance
/// that borrowing is occurring. This lint ensures that lifetime
/// parameters are always explicitly stated, even if it is the `'_`
/// [placeholder lifetime].
///
/// This lint is "allow" by default because it has some known issues, and
/// may require a significant transition for old code.
///
/// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
pub ELIDED_LIFETIMES_IN_PATHS_UNTIED,
Allow,
"hidden lifetime parameters in types make it hard to tell when borrowing is happening",
crate_level_only
}

declare_lint! {
/// The `elided_named_lifetimes` lint detects when an elided
/// lifetime ends up being a named lifetime, such as `'static`
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,10 @@ pub enum BuiltinLintDiag {
},
MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
ElidedLifetimesInPaths(usize, Span, bool, Span),
ElidedLifetimesInPathsTied {
elided_lifetime_source: Option<Span>,
suggestions: Vec<(Span, usize, bool)>,
},
ElidedNamedLifetimes {
elided: (Span, MissingLifetimeKind),
resolution: ElidedLifetimeResolution,
Expand Down
Loading

0 comments on commit 50802c5

Please sign in to comment.