Skip to content

Commit f6d1275

Browse files
committed
Split elided_lifetime_in_paths into tied and untied
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.
1 parent a985926 commit f6d1275

File tree

11 files changed

+582
-30
lines changed

11 files changed

+582
-30
lines changed

compiler/rustc_lint/src/lib.rs

+11-2
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,8 @@ fn register_builtins(store: &mut LintStore) {
303303
BARE_TRAIT_OBJECTS,
304304
UNUSED_EXTERN_CRATES,
305305
ELLIPSIS_INCLUSIVE_RANGE_PATTERNS,
306-
ELIDED_LIFETIMES_IN_PATHS,
306+
ELIDED_LIFETIMES_IN_PATHS_TIED,
307+
ELIDED_LIFETIMES_IN_PATHS_UNTIED,
307308
EXPLICIT_OUTLIVES_REQUIREMENTS,
308309
// FIXME(#52665, #47816) not always applicable and not all
309310
// macros are ready for this yet.
@@ -313,9 +314,14 @@ fn register_builtins(store: &mut LintStore) {
313314
// MACRO_USE_EXTERN_CRATE
314315
);
315316

317+
add_lint_group!(
318+
"elided_lifetimes_in_paths",
319+
ELIDED_LIFETIMES_IN_PATHS_TIED,
320+
ELIDED_LIFETIMES_IN_PATHS_UNTIED,
321+
);
322+
316323
// Register renamed and removed lints.
317324
store.register_renamed("single_use_lifetime", "single_use_lifetimes");
318-
store.register_renamed("elided_lifetime_in_path", "elided_lifetimes_in_paths");
319325
store.register_renamed("bare_trait_object", "bare_trait_objects");
320326
store.register_renamed("unstable_name_collision", "unstable_name_collisions");
321327
store.register_renamed("unused_doc_comment", "unused_doc_comments");
@@ -329,6 +335,9 @@ fn register_builtins(store: &mut LintStore) {
329335
store.register_renamed("unused_tuple_struct_fields", "dead_code");
330336
store.register_renamed("static_mut_ref", "static_mut_refs");
331337

338+
// Register renamed lint groups
339+
store.register_renamed_group("elided_lifetime_in_path", "elided_lifetimes_in_paths");
340+
332341
// These were moved to tool lints, but rustc still sees them when compiling normally, before
333342
// tool lints are registered, so `check_tool_name_for_backwards_compat` doesn't work. Use
334343
// `register_removed` explicitly.

compiler/rustc_lint_defs/src/builtin.rs

+48-6
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ declare_lint_pass! {
3939
DEPRECATED_WHERE_CLAUSE_LOCATION,
4040
DUPLICATE_MACRO_ATTRIBUTES,
4141
ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
42-
ELIDED_LIFETIMES_IN_PATHS,
42+
ELIDED_LIFETIMES_IN_PATHS_TIED,
43+
ELIDED_LIFETIMES_IN_PATHS_UNTIED,
4344
EXPORTED_PRIVATE_DEPENDENCIES,
4445
FFI_UNWIND_CALLS,
4546
FORBIDDEN_LINT_GROUPS,
@@ -1730,19 +1731,21 @@ declare_lint! {
17301731
}
17311732

17321733
declare_lint! {
1733-
/// The `elided_lifetimes_in_paths` lint detects the use of hidden
1734-
/// lifetime parameters.
1734+
/// The `elided_lifetimes_in_paths_tied` lint detects the use of
1735+
/// hidden lifetime parameters when those lifetime parameters tie
1736+
/// an input lifetime parameter to an output lifetime parameter.
17351737
///
17361738
/// ### Example
17371739
///
17381740
/// ```rust,compile_fail
1739-
/// #![deny(elided_lifetimes_in_paths)]
1741+
/// #![deny(elided_lifetimes_in_paths_tied)]
17401742
/// #![deny(warnings)]
17411743
/// struct Foo<'a> {
17421744
/// x: &'a u32
17431745
/// }
17441746
///
1745-
/// fn foo(x: &Foo) {
1747+
/// fn foo(x: Foo) -> &u32 {
1748+
/// &x.0
17461749
/// }
17471750
/// ```
17481751
///
@@ -1759,12 +1762,51 @@ declare_lint! {
17591762
/// may require a significant transition for old code.
17601763
///
17611764
/// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
1762-
pub ELIDED_LIFETIMES_IN_PATHS,
1765+
pub ELIDED_LIFETIMES_IN_PATHS_TIED,
17631766
Allow,
17641767
"hidden lifetime parameters in types are deprecated",
17651768
crate_level_only
17661769
}
17671770

1771+
declare_lint! {
1772+
/// The `elided_lifetimes_in_paths_untied` lint detects the use of
1773+
/// hidden lifetime parameters when those lifetime parameters do
1774+
/// not tie an input lifetime parameter to an output lifetime
1775+
/// parameter.
1776+
///
1777+
/// ### Example
1778+
///
1779+
/// ```rust,compile_fail
1780+
/// #![deny(elided_lifetimes_in_paths_untied)]
1781+
/// #![deny(warnings)]
1782+
/// struct Foo<'a> {
1783+
/// x: &'a u32
1784+
/// }
1785+
///
1786+
/// fn foo(x: Foo) -> u32 {
1787+
/// x.0
1788+
/// }
1789+
/// ```
1790+
///
1791+
/// {{produces}}
1792+
///
1793+
/// ### Explanation
1794+
///
1795+
/// Elided lifetime parameters can make it difficult to see at a glance
1796+
/// that borrowing is occurring. This lint ensures that lifetime
1797+
/// parameters are always explicitly stated, even if it is the `'_`
1798+
/// [placeholder lifetime].
1799+
///
1800+
/// This lint is "allow" by default because it has some known issues, and
1801+
/// may require a significant transition for old code.
1802+
///
1803+
/// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
1804+
pub ELIDED_LIFETIMES_IN_PATHS_UNTIED,
1805+
Allow,
1806+
"hidden lifetime parameters in types make it hard to tell when borrowing is happening",
1807+
crate_level_only
1808+
}
1809+
17681810
declare_lint! {
17691811
/// The `bare_trait_objects` lint suggests using `dyn Trait` for trait
17701812
/// objects.

compiler/rustc_resolve/src/late.rs

+99-20
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,24 @@ struct DiagMetadata<'ast> {
646646
}
647647

648648
#[derive(Debug)]
649-
struct ResolvedNestedElisionTarget {
649+
enum ResolvedElisionTarget {
650+
/// Elision in `&u8` -> `&'_ u8`
651+
TopLevel(NodeId),
652+
/// Elision in `Foo` -> `Foo<'_>`
653+
Nested(NestedResolvedElisionTarget),
654+
}
655+
656+
impl ResolvedElisionTarget {
657+
fn node_id(&self) -> NodeId {
658+
match *self {
659+
Self::TopLevel(n) => n,
660+
Self::Nested(NestedResolvedElisionTarget { segment_id, .. }) => segment_id,
661+
}
662+
}
663+
}
664+
665+
#[derive(Debug)]
666+
struct NestedResolvedElisionTarget {
650667
segment_id: NodeId,
651668
elided_lifetime_span: Span,
652669
diagnostic: lint::BuiltinLintDiag,
@@ -694,7 +711,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
694711
lifetime_uses: FxHashMap<LocalDefId, LifetimeUseSet>,
695712

696713
/// Track which types participated in lifetime elision
697-
resolved_lifetime_elisions: Vec<ResolvedNestedElisionTarget>,
714+
resolved_lifetime_elisions: Vec<ResolvedElisionTarget>,
698715
}
699716

700717
/// Walks the whole crate in DFS order, visiting each item, resolving names as it goes.
@@ -1742,6 +1759,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
17421759
LifetimeElisionCandidate::Ignore,
17431760
);
17441761
self.resolve_anonymous_lifetime(&lt, true);
1762+
1763+
self.resolved_lifetime_elisions.push(ResolvedElisionTarget::TopLevel(anchor_id));
17451764
}
17461765

17471766
#[instrument(level = "debug", skip(self))]
@@ -1953,16 +1972,18 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
19531972
}
19541973

19551974
if should_lint {
1956-
self.resolved_lifetime_elisions.push(ResolvedNestedElisionTarget {
1957-
segment_id,
1958-
elided_lifetime_span,
1959-
diagnostic: lint::BuiltinLintDiag::ElidedLifetimesInPaths(
1960-
expected_lifetimes,
1961-
path_span,
1962-
!segment.has_generic_args,
1975+
self.resolved_lifetime_elisions.push(ResolvedElisionTarget::Nested(
1976+
NestedResolvedElisionTarget {
1977+
segment_id,
19631978
elided_lifetime_span,
1964-
),
1965-
});
1979+
diagnostic: lint::BuiltinLintDiag::ElidedLifetimesInPaths(
1980+
expected_lifetimes,
1981+
path_span,
1982+
!segment.has_generic_args,
1983+
elided_lifetime_span,
1984+
),
1985+
},
1986+
));
19661987
}
19671988
}
19681989
}
@@ -2024,22 +2045,80 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
20242045

20252046
let elision_failures =
20262047
replace(&mut self.diag_metadata.current_elision_failures, outer_failures);
2027-
if !elision_failures.is_empty() {
2028-
let Err(failure_info) = elision_lifetime else { bug!() };
2029-
self.report_missing_lifetime_specifiers(elision_failures, Some(failure_info));
2030-
}
2048+
2049+
let elision_lifetime = match (elision_failures.is_empty(), elision_lifetime) {
2050+
(true, Ok(lifetime)) => Some(lifetime),
2051+
2052+
(true, Err(_)) => None,
2053+
2054+
(false, Ok(_)) => bug!(),
2055+
2056+
(false, Err(failure_info)) => {
2057+
self.report_missing_lifetime_specifiers(elision_failures, Some(failure_info));
2058+
None
2059+
}
2060+
};
2061+
2062+
// We've recorded all elisions that occurred in the params and
2063+
// outputs, categorized by top-level or nested.
2064+
//
2065+
// Our primary lint case is when an output lifetime is tied to
2066+
// an input lifetime. In that case, we want to warn about any
2067+
// nested hidden lifetimes in those params or outputs.
2068+
//
2069+
// The secondary case is for nested elisions that are not part
2070+
// of the tied lifetime relationship.
20312071

20322072
let output_resolved_lifetime_elisions =
20332073
replace(&mut self.resolved_lifetime_elisions, outer_resolved_lifetime_elisions);
20342074

2035-
let resolved_lifetime_elisions =
2036-
param_resolved_lifetime_elisions.into_iter().chain(output_resolved_lifetime_elisions);
2075+
match (output_resolved_lifetime_elisions.is_empty(), elision_lifetime) {
2076+
(true, _) | (_, None) => {
2077+
// Treat all parameters as untied
2078+
self.report_elided_lifetimes_in_paths(
2079+
param_resolved_lifetime_elisions,
2080+
lint::builtin::ELIDED_LIFETIMES_IN_PATHS_UNTIED,
2081+
);
2082+
}
2083+
(false, Some(elision_lifetime)) => {
2084+
let (primary, secondary): (Vec<_>, Vec<_>) =
2085+
param_resolved_lifetime_elisions.into_iter().partition(|re| {
2086+
// Recover the `NodeId` of an elided lifetime
2087+
let lvl1 = &self.r.lifetimes_res_map[&re.node_id()];
2088+
let lvl2 = match lvl1 {
2089+
LifetimeRes::ElidedAnchor { start, .. } => {
2090+
&self.r.lifetimes_res_map[&start]
2091+
}
2092+
o => o,
2093+
};
2094+
2095+
lvl2 == &elision_lifetime
2096+
});
2097+
2098+
self.report_elided_lifetimes_in_paths(
2099+
primary.into_iter().chain(output_resolved_lifetime_elisions),
2100+
lint::builtin::ELIDED_LIFETIMES_IN_PATHS_TIED,
2101+
);
2102+
self.report_elided_lifetimes_in_paths(
2103+
secondary,
2104+
lint::builtin::ELIDED_LIFETIMES_IN_PATHS_UNTIED,
2105+
);
2106+
}
2107+
}
2108+
}
2109+
2110+
fn report_elided_lifetimes_in_paths(
2111+
&mut self,
2112+
resolved_elisions: impl IntoIterator<Item = ResolvedElisionTarget>,
2113+
lint: &'static lint::Lint,
2114+
) {
2115+
for re in resolved_elisions {
2116+
let ResolvedElisionTarget::Nested(d) = re else { continue };
20372117

2038-
for re in resolved_lifetime_elisions {
2039-
let ResolvedNestedElisionTarget { segment_id, elided_lifetime_span, diagnostic } = re;
2118+
let NestedResolvedElisionTarget { segment_id, elided_lifetime_span, diagnostic } = d;
20402119

20412120
self.r.lint_buffer.buffer_lint_with_diagnostic(
2042-
lint::builtin::ELIDED_LIFETIMES_IN_PATHS,
2121+
lint,
20432122
segment_id,
20442123
elided_lifetime_span,
20452124
"hidden lifetime parameters in types are deprecated",

src/tools/lint-docs/src/groups.rs

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[
1111
("let-underscore", "Lints that detect wildcard let bindings that are likely to be invalid"),
1212
("rustdoc", "Rustdoc-specific lints"),
1313
("rust-2018-idioms", "Lints to nudge you toward idiomatic features of Rust 2018"),
14+
("elided-lifetimes-in-paths", "Lints that detect the use of hidden lifetime parameters"),
1415
("nonstandard-style", "Violation of standard naming conventions"),
1516
("future-incompatible", "Lints that detect code that has future-compatibility problems"),
1617
("rust-2018-compatibility", "Lints used to transition code from the 2015 edition to 2018"),

0 commit comments

Comments
 (0)