Skip to content

Commit da16b9b

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 21be4f5 commit da16b9b

File tree

11 files changed

+582
-30
lines changed

11 files changed

+582
-30
lines changed

Diff for: 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");
@@ -328,6 +334,9 @@ fn register_builtins(store: &mut LintStore) {
328334
store.register_renamed("non_fmt_panic", "non_fmt_panics");
329335
store.register_renamed("unused_tuple_struct_fields", "dead_code");
330336

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

Diff for: compiler/rustc_lint_defs/src/builtin.rs

+48-6
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ declare_lint_pass! {
4040
DEPRECATED_WHERE_CLAUSE_LOCATION,
4141
DUPLICATE_MACRO_ATTRIBUTES,
4242
ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
43-
ELIDED_LIFETIMES_IN_PATHS,
43+
ELIDED_LIFETIMES_IN_PATHS_TIED,
44+
ELIDED_LIFETIMES_IN_PATHS_UNTIED,
4445
EXPORTED_PRIVATE_DEPENDENCIES,
4546
FFI_UNWIND_CALLS,
4647
FORBIDDEN_LINT_GROUPS,
@@ -1701,19 +1702,21 @@ declare_lint! {
17011702
}
17021703

17031704
declare_lint! {
1704-
/// The `elided_lifetimes_in_paths` lint detects the use of hidden
1705-
/// lifetime parameters.
1705+
/// The `elided_lifetimes_in_paths_tied` lint detects the use of
1706+
/// hidden lifetime parameters when those lifetime parameters tie
1707+
/// an input lifetime parameter to an output lifetime parameter.
17061708
///
17071709
/// ### Example
17081710
///
17091711
/// ```rust,compile_fail
1710-
/// #![deny(elided_lifetimes_in_paths)]
1712+
/// #![deny(elided_lifetimes_in_paths_tied)]
17111713
/// #![deny(warnings)]
17121714
/// struct Foo<'a> {
17131715
/// x: &'a u32
17141716
/// }
17151717
///
1716-
/// fn foo(x: &Foo) {
1718+
/// fn foo(x: Foo) -> &u32 {
1719+
/// &x.0
17171720
/// }
17181721
/// ```
17191722
///
@@ -1730,12 +1733,51 @@ declare_lint! {
17301733
/// may require a significant transition for old code.
17311734
///
17321735
/// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
1733-
pub ELIDED_LIFETIMES_IN_PATHS,
1736+
pub ELIDED_LIFETIMES_IN_PATHS_TIED,
17341737
Allow,
17351738
"hidden lifetime parameters in types are deprecated",
17361739
crate_level_only
17371740
}
17381741

1742+
declare_lint! {
1743+
/// The `elided_lifetimes_in_paths_untied` lint detects the use of
1744+
/// hidden lifetime parameters when those lifetime parameters do
1745+
/// not tie an input lifetime parameter to an output lifetime
1746+
/// parameter.
1747+
///
1748+
/// ### Example
1749+
///
1750+
/// ```rust,compile_fail
1751+
/// #![deny(elided_lifetimes_in_paths_untied)]
1752+
/// #![deny(warnings)]
1753+
/// struct Foo<'a> {
1754+
/// x: &'a u32
1755+
/// }
1756+
///
1757+
/// fn foo(x: Foo) -> u32 {
1758+
/// x.0
1759+
/// }
1760+
/// ```
1761+
///
1762+
/// {{produces}}
1763+
///
1764+
/// ### Explanation
1765+
///
1766+
/// Elided lifetime parameters can make it difficult to see at a glance
1767+
/// that borrowing is occurring. This lint ensures that lifetime
1768+
/// parameters are always explicitly stated, even if it is the `'_`
1769+
/// [placeholder lifetime].
1770+
///
1771+
/// This lint is "allow" by default because it has some known issues, and
1772+
/// may require a significant transition for old code.
1773+
///
1774+
/// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
1775+
pub ELIDED_LIFETIMES_IN_PATHS_UNTIED,
1776+
Allow,
1777+
"hidden lifetime parameters in types make it hard to tell when borrowing is happening",
1778+
crate_level_only
1779+
}
1780+
17391781
declare_lint! {
17401782
/// The `bare_trait_objects` lint suggests using `dyn Trait` for trait
17411783
/// objects.

Diff for: compiler/rustc_resolve/src/late.rs

+99-20
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,24 @@ struct DiagnosticMetadata<'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::BuiltinLintDiagnostics,
@@ -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.
@@ -1745,6 +1762,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
17451762
LifetimeElisionCandidate::Ignore,
17461763
);
17471764
self.resolve_anonymous_lifetime(&lt, true);
1765+
1766+
self.resolved_lifetime_elisions.push(ResolvedElisionTarget::TopLevel(anchor_id));
17481767
}
17491768

17501769
#[instrument(level = "debug", skip(self))]
@@ -1957,16 +1976,18 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
19571976
}
19581977

19591978
if should_lint {
1960-
self.resolved_lifetime_elisions.push(ResolvedNestedElisionTarget {
1961-
segment_id,
1962-
elided_lifetime_span,
1963-
diagnostic: lint::BuiltinLintDiagnostics::ElidedLifetimesInPaths(
1964-
expected_lifetimes,
1965-
path_span,
1966-
!segment.has_generic_args,
1979+
self.resolved_lifetime_elisions.push(ResolvedElisionTarget::Nested(
1980+
NestedResolvedElisionTarget {
1981+
segment_id,
19671982
elided_lifetime_span,
1968-
),
1969-
});
1983+
diagnostic: lint::BuiltinLintDiagnostics::ElidedLifetimesInPaths(
1984+
expected_lifetimes,
1985+
path_span,
1986+
!segment.has_generic_args,
1987+
elided_lifetime_span,
1988+
),
1989+
},
1990+
));
19701991
}
19711992
}
19721993
}
@@ -2028,22 +2049,80 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
20282049

20292050
let elision_failures =
20302051
replace(&mut self.diagnostic_metadata.current_elision_failures, outer_failures);
2031-
if !elision_failures.is_empty() {
2032-
let Err(failure_info) = elision_lifetime else { bug!() };
2033-
self.report_missing_lifetime_specifiers(elision_failures, Some(failure_info));
2034-
}
2052+
2053+
let elision_lifetime = match (elision_failures.is_empty(), elision_lifetime) {
2054+
(true, Ok(lifetime)) => Some(lifetime),
2055+
2056+
(true, Err(_)) => None,
2057+
2058+
(false, Ok(_)) => bug!(),
2059+
2060+
(false, Err(failure_info)) => {
2061+
self.report_missing_lifetime_specifiers(elision_failures, Some(failure_info));
2062+
None
2063+
}
2064+
};
2065+
2066+
// We've recorded all elisions that occurred in the params and
2067+
// outputs, categorized by top-level or nested.
2068+
//
2069+
// Our primary lint case is when an output lifetime is tied to
2070+
// an input lifetime. In that case, we want to warn about any
2071+
// parameters that had a nested elision.
2072+
//
2073+
// The secondary case is for nested elisions that are not part
2074+
// of the tied lifetime relationship.
20352075

20362076
let output_resolved_lifetime_elisions =
20372077
replace(&mut self.resolved_lifetime_elisions, outer_resolved_lifetime_elisions);
20382078

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

2042-
for re in resolved_lifetime_elisions {
2043-
let ResolvedNestedElisionTarget { segment_id, elided_lifetime_span, diagnostic } = re;
2122+
let NestedResolvedElisionTarget { segment_id, elided_lifetime_span, diagnostic } = d;
20442123

20452124
self.r.lint_buffer.buffer_lint_with_diagnostic(
2046-
lint::builtin::ELIDED_LIFETIMES_IN_PATHS,
2125+
lint,
20472126
segment_id,
20482127
elided_lifetime_span,
20492128
"hidden lifetime parameters in types are deprecated",

Diff for: 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)