Split elided_lifetime_in_paths into finer-grained lints by shepmaster · Pull Request #120808 · rust-lang/rust

shepmaster · 2024-02-08T19:40:38Z

Description

Converts the existing elided_lifetime_in_paths lint into three smaller pieces:

hidden_lifetimes_in_input_paths — fires for fn(ContainsLifetime) -> ...
hidden_lifetimes_in_output_paths — fires for fn(...) -> ContainsLifetime
hidden_lifetimes_in_type_paths — fires for many other usages of ContainsLifetime, such as in a static or in a turbofish

A new group hidden_lifetimes_in_paths is created with the three smaller lints and places that use the old elided_lifetime_in_paths name are updated to match.

#![allow(dead_code)]
#![warn(hidden_lifetimes_in_paths)]

struct ContainsLifetime<'a>(&'a u8);

fn inputs_and_outputs(v: ContainsLifetime) -> ContainsLifetime { v }
static CL: ContainsLifetime = ContainsLifetime(&42);

fn main() {}

warning: paths containing hidden lifetime parameters are deprecated
 --> a.rs:6:26
  |
6 | fn inputs_and_outputs(v: ContainsLifetime) -> ContainsLifetime { v }
  |                          ^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(hidden_lifetimes_in_input_paths)]` implied by `#[warn(hidden_lifetimes_in_paths)]`
help: indicate the anonymous lifetime
  |
6 | fn inputs_and_outputs(v: ContainsLifetime<'_>) -> ContainsLifetime { v }
  |                                          ++++

warning: paths containing hidden lifetime parameters are deprecated
 --> a.rs:6:47
  |
6 | fn inputs_and_outputs(v: ContainsLifetime) -> ContainsLifetime { v }
  |                                               ^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(hidden_lifetimes_in_output_paths)]` implied by `#[warn(hidden_lifetimes_in_paths)]`
help: indicate the anonymous lifetime
  |
6 | fn inputs_and_outputs(v: ContainsLifetime) -> ContainsLifetime<'_> { v }
  |                                                               ++++

warning: paths containing hidden lifetime parameters are deprecated
 --> a.rs:7:12
  |
7 | static CL: ContainsLifetime = ContainsLifetime(&42);
  |            ^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(hidden_lifetimes_in_type_paths)]` implied by `#[warn(hidden_lifetimes_in_paths)]`
help: indicate the anonymous lifetime
  |
7 | static CL: ContainsLifetime<'_> = ContainsLifetime(&42);
  |                            ++++

Background

In general, we want to discourage function signatures like fn (&T) -> ContainsLifetime, fn (ContainsLifetime) -> &T , and fn (ContainsLifetime) -> ContainsLifetime as it is not obvious that a lifetime flows through the function and back out as there is no visual indication that ContainsLifetime has a lifetime generic (such types are not usually literally called "contains lifetime" 😃).

In #120808 (comment) (and multiple followup comments, e.g. #120808 (comment)), the lang team decided on a plan where we introduce a new warn-by-default lint mismatched_lifetime_syntaxes (superseding elided_named_lifetimes) which helps insure consistency between input and output lifetime syntaxes and then split elided_lifetime_in_paths into three parts. The combination of these lints should be enough to accomplish the original goal.

History

Note that this PR has substantially changed from its original version. Check the (copious) comments below as well as the edit history of this message.

rustbot · 2024-02-08T19:40:50Z

r? @pnkfelix

rustbot has assigned @pnkfelix.
They will have a look at your PR within the next two weeks and either review your PR or
reassign to another reviewer.

Use r? to explicitly pick a reviewer

compiler/rustc_lint/src/lib.rs

tests/ui/lifetimes/elided-lifetime-in-path-details.rs

shepmaster · 2024-02-09T19:39:19Z

As I've now tried to add this test twice and to help prevent trying to add it again... this fails because elision can't take place:

fn top_level_nested_to_top_level_nested(v: &ContainsLifetime) -> &ContainsLifetime { v }

error[E0106]: missing lifetime specifiers
 --> src/lib.rs:3:66
  |
3 | fn top_level_nested_to_top_level_nested(v: &ContainsLifetime) -> &ContainsLifetime {
  |                                            -----------------     ^^^^^^^^^^^^^^^^^ expected named lifetime parameter
  |                                                                  |
  |                                                                  expected named lifetime parameter
  |

compiler/rustc_lint_defs/src/builtin.rs

compiler/rustc_resolve/src/late.rs

tests/ui/lifetimes/elided-lifetime-in-path-details.rs

compiler/rustc_lint/src/lib.rs

tests/ui/lifetimes/elided-lifetime-in-path-details.rs

pnkfelix · 2024-02-12T20:12:40Z

This generally looks fine. I had a few questions about what we expect to happen in a corner case.

~~r=me once those questions are addressed in some way~~ (update: well, maybe the r=me is premature given the list of questions that @shepmaster included in the PR description. But I don't think the PR is waiting on review at this point.)

@rustbot label: +S-waiting-on-author -S-waiting-on-review

pnkfelix · 2024-02-12T20:14:02Z

Oh, I suppose there is still an open question about the use of the "tied"/"untied" terminology, which I admit threw me for a loop at first. I'm not sure which group is the best to handle resolving that question, though. And I'm also not entirely sure that resolving that question should block landing this work.

Is resolving a question like that a matter for WG-diagnostics, or for T-lang?

shepmaster · 2024-02-12T20:37:51Z

Is resolving a question like that a matter for WG-diagnostics, or for T-lang?

That's a great question that I don't have an answer for. I posed it in the Zulip thread hoping there was some existing terminology. Unfortunately, no one seemed aware of one. "Tied" made some intuitive sense for the small handful of people I asked one-on-one.

It feels like this is something that we must have talked about before and potentially even documented somewhere, but 🤷

compiler/rustc_lint/src/lib.rs

danielhenrymantilla

Thanks for pushing this forward! ❤️

shepmaster · 2025-06-14T21:16:15Z

r? @jieyouxu

rustbot · 2025-06-14T21:16:21Z

Changes to the size of AST and/or HIR nodes.

cc @nnethercote

nnethercote · 2025-06-15T00:53:35Z

compiler/rustc_hir/src/hir.rs

    static_assert_size!(TraitItem<'_>, 88);
    static_assert_size!(TraitItemKind<'_>, 48);
-    static_assert_size!(Ty<'_>, 48);
+    static_assert_size!(Ty<'_>, 56);


Ah, this is unfortunate. Ty is a very common type, an extra 8 bytes just to fit a boolean?

jieyouxu

Thanks for working on this! The broad implementation looks good to me modulo some nits, however I have two implementation specifics that I'm not very sure about.

Here's my initial review pass, but I would like to ask for a second opinion from @oli-obk (or @nnethercote) regarding two questions:

The "overlapping" mismatched_lifetime_syntaxes and hidden_lifetimes_in_output_paths situation, and whether to perform some kind of "suppression" to have mismatched_lifetime_syntaxes take precedence over hidden_lifetimes_in_output_paths unless hidden_lifetimes_in_output_paths have a stronger lint level. And if that is desired, the more important question is how, since these two lints run in their own lint passes. AFAICT, it doesn't seem easy to stash both lint diagnostics and their respective effective lint levels, then perform some kind of post-diagnostics-construction precedence filter that accounts for the relative lint levels between the two lints.
I'm also not very sure about the increase in size of Ty<'_>, since this is a very commonly used type, as @nnethercote pointed out in an earlier review comment. It seems like tracking source of the type is desirable, but I'm not too sure about an alternative scheme that doesn't involve increasing size of Ty<'_>.

I'm going to hand this review off to oli for a second opinion.

jieyouxu · 2025-06-19T02:08:16Z

tests/ui/lifetimes/elided-lifetimes-in-type-paths.rs

+// signatures, but they can also appear in other places! The original
+// version of this lint handled all these cases in one location, but
+// it's desired that the one lint actually be multiple.


Nit (wording): maybe for clarity (I was reading this, and it wasn't immediately clear)

Most of the time, we focus on elided lifetimes in function signatures, but they can also appear in other places! The original lint elided_lifetime_in_paths handled all these cases in one location, but it's more desirable to split it into 3 more fine-grained lints under a new hidden_lifetimes_in_paths lint group umbrella:

hidden_lifetimes_in_input_paths

hidden_lifetimes_in_output_paths

hidden_lifetimes_in_type_paths

jieyouxu · 2025-06-19T02:20:20Z

compiler/rustc_lint/messages.ftl

+lint_hidden_lifetime_in_path =
+    paths containing hidden lifetime parameters are deprecated


Suggestion [WORDING-STRENGTH 1/2]: I think I agree with @tmandry's comment earlier

hidden lifetime parameters in types are deprecated

I also think we should remove this wording at least until we upgrade some lint to warn-by-default.

where if this set of 3 lints are initially allow-by-default in this PR, we should not use "deprecated" in this PR, but instead add "deprecated" in the follow-up PR that bumps these three lints and the hidden_lifetimes_in_paths to warn-by-default, so that the deprecation is part of the follow-up decision to explicitly deprecate this form.

In this PR, I would reword this less strong initially, maybe something like

paths containing hidden lifetime parameters can be confusing

We're still staying "deprecated" here. While at some level I do think that's correct, I'd highlight the comment by @tmandry that @jieyouxu highlighted above; there'd need to be a reply to this to argue for why it should be kept. We would almost certainly get pushback on this language.

jieyouxu · 2025-06-19T02:27:22Z

compiler/rustc_lint/src/lifetime_syntax.rs

+    /// explicitly stated, even if it is the `'_` [placeholder
+    /// lifetime].


Nit (wording): To make it easier to search for '_, maybe

This lint ensures that lifetime parameters are always explicitly stated, even if it is the placeholder lifetime '_.

Actually hm, this is more so trying to express

This lint ensures that lifetime parameters are explicit in types occuring as function arguments. Where lifetime elision may occur, the placeholder lifetime '_ may be used to make the presence of lifetime constraints explicit.

jieyouxu · 2025-06-19T02:36:40Z

compiler/rustc_lint/src/lifetime_syntax.rs

+    /// glance that borrowing is occurring. This is especially true
+    /// when a type is used as a function's return value: lifetime
+    /// elision will link the return value's lifetime to an argument's
+    /// lifetime, but no syntax in the function signature indicates
+    /// that.


Discussion: right, I see what you meant by having both mismatched_lifetime_syntaxes and hidden_lifetimes_in_output_paths firing against the same return position type-with-hidden-lifetime.

For reference, the discussion about what a reasonable scheme is to suppress the lower-lint-level of the two lints, or have mismatched_lifetime_syntaxes "shadow" hidden_lifetimes_in_output_paths when both are at the same lint level is at #t-compiler/diagnostics > Avoiding emitting one lint if another has been emitted.

@oli-obk suggested in that thread something among the lines of

Since we already have stashed errors we could have lint marking where we can explicitly mark spans as having linted and other lints can check if spans have been linted

Tho I'm not sure how to cleanly massage e.g. the stashed diagnostics mechanism and StashKey to indicate how to resolve if both lints are to be emitted (since they'd be ran in separate lint passes). I'll hand the review over to oli for this part.

my intention was that we'd add a new system, but I do realize now it's not incremental friendly (which isn't an issue for errors).

jieyouxu · 2025-06-19T02:39:49Z

compiler/rustc_lint/src/lifetime_syntax.rs

+    /// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
+    pub HIDDEN_LIFETIMES_IN_TYPE_PATHS,
+    Allow,
+    "hidden lifetime parameters in types outside function signatures are discouraged"


Suggestion [WORDING-STRENGTH 2/2]: same here, maybe lessen the "strength" conveyed in this initial PR adding this lint as allow-by-default, and defer the lint level bump + wording strengthening (to "deprecate" and "discourage") in the follow-up upgrade-to-warn-by-default PR. I.e. maybe

hidden lifetime parameters in types outside function signatures can be confusing

But I don't feel too strongly about this.

jieyouxu · 2025-06-19T02:44:31Z

tests/ui/lifetimes/hidden-lifetimes-in-mod.rs

+#[deny(hidden_lifetimes_in_paths)]
+mod w {
+    fn test2(_: super::Foo) {}
+    //~^ ERROR paths containing hidden lifetime parameters are deprecated
+}


Suggestion: can you add a little more elaboration on what this test intends to exercise? Is it to check that we recurse into items with mods, and/or the lint level can be properly controlled through lint level attributes?

jieyouxu · 2025-06-19T02:57:50Z

r? @oli-obk (for a second opinion, unless you're busy)

briansmith · 2025-06-22T17:40:33Z

The tests should include the following cases as well, with the lints enabled:

struct ContainsLifetime<'a>(&'a u8);

impl ContainsLifetime<'_> {
   fn self_borrowed_self_output(&self) -> Self { unimplemented!() }
   fn self_moved_output(self) -> Self { self }
   fn self_another_borrowed_self_output(self, _: &u8) -> Self { self }
   fn self_input(&self, _: &Self) { }
   fn self_arbitrary(self: Self) { }
   fn self_arbitrary_self(self: Self) -> Self { self }
}

This will prevent us from accidentally changing whether Self is considered to have hidden lifetimes.

cjgillot · 2025-06-22T19:52:03Z

compiler/rustc_hir/src/hir.rs

+    /// Details not yet needed. Feel free to give useful
+    /// categorization to these usages.
+    Other,
+}


Can we avoid having this enum and have the lint compute where the type appears?

Specifically: you can look at the parent, or implement the lint on paths and look at the first segment

I don't know how to do this to achieve the goals. Interested parties can watch as I flail about over in Zulip.

bors · 2025-06-24T18:54:17Z

☔ The latest upstream changes (presumably #142962) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk · 2025-07-25T18:37:08Z

@rustbot author

rustbot · 2025-08-04T03:00:51Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in need_type_info.rs

cc @lcnr

HIR ty lowering was modified

cc @fmease

rustbot · 2025-08-04T03:23:29Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Removing the `issue-91763` test as the implementation is completely different now. Bootstrap forces `rust_2018_idioms` to the warning level in the rustc_lint doctests using `-Zcrate-attr`. This overrides the doctest's crate-level `deny` attributes, so I've changed those to be statement-level attributes.

shepmaster · 2025-08-11T14:57:12Z

@rustbot ready

traviscross · 2025-08-11T19:20:49Z

compiler/rustc_lint/messages.ftl

+    paths containing hidden lifetime parameters are deprecated
+
+lint_hidden_lifetime_in_path_suggestion =
+    indicate the anonymous lifetime


Suggested change

indicate the anonymous lifetime

indicate the inferred lifetime

I'd probably say "inferred" here for a couple reasons. One, I'm planning to update the Reference to call this the inferred lifetime (as we call _ the inferred type/const). Two, the lifetime isn't necessarily anonymous. It could be inferred to be a named lifetime, though of course we lint against that separately.

traviscross · 2025-08-11T19:26:53Z

compiler/rustc_lint/src/lifetime_syntax.rs

+    /// The `hidden_lifetimes_in_output_paths` lint detects the use
+    /// of hidden lifetime parameters in types occurring as a function
+    /// return value.


Suggested change

/// The `hidden_lifetimes_in_output_paths` lint detects the use

/// of hidden lifetime parameters in types occurring as a function

/// return value.

/// The `hidden_lifetimes_in_output_paths` lint detects use of

/// hidden lifetime parameters in the return type of a function.

traviscross · 2025-08-11T19:32:46Z

compiler/rustc_lint/src/lifetime_syntax.rs

+    /// Hidden lifetime parameters can make it difficult to see at a
+    /// glance that borrowing is occurring. This is especially true
+    /// when a type is used as a function's return value: lifetime
+    /// elision will link the return value's lifetime to an argument's
+    /// lifetime, but no syntax in the function signature indicates
+    /// that.


Suggested change

/// Hidden lifetime parameters can make it difficult to see at a

/// glance that borrowing is occurring. This is especially true

/// when a type is used as a function's return value: lifetime

/// elision will link the return value's lifetime to an argument's

/// lifetime, but no syntax in the function signature indicates

/// that.

/// Hidden lifetime parameters can make it difficult to see at a

/// glance that borrowing is occurring. This is especially true

/// when the return type of a function hides a lifetime. Lifetime

/// elision will link the lifetime in the return type to an input

/// lifetime without any indication of this in the signature.

(This is to clean up (or avoid) some terminology around types/values and arguments/parameters.)

traviscross · 2025-08-11T20:22:00Z

compiler/rustc_lint/src/lifetime_syntax.rs

+    /// The `hidden_lifetimes_in_type_paths` lint detects the use of
+    /// hidden lifetime parameters in types not part of a function's
+    /// arguments or return values.


Suggested change

/// The `hidden_lifetimes_in_type_paths` lint detects the use of

/// hidden lifetime parameters in types not part of a function's

/// arguments or return values.

/// The `hidden_lifetimes_in_type_paths` lint detects the use of

/// hidden lifetime parameters in types other than those used

/// within parameters or within the return type of a function.

traviscross · 2025-08-11T21:41:04Z

compiler/rustc_lint/messages.ftl

+lint_hidden_lifetime_in_path =
+    paths containing hidden lifetime parameters are deprecated


We're still staying "deprecated" here. While at some level I do think that's correct, I'd highlight the comment by @tmandry that @jieyouxu highlighted above; there'd need to be a reply to this to argue for why it should be kept. We would almost certainly get pushback on this language.

traviscross · 2025-08-11T21:46:24Z

Since this thread has become long and there were multiple FCP proposals, I'll mention for @oli-obk's convenience the one we're reviewing this against is #120808 (comment).

bors · 2025-08-13T22:58:00Z

☔ The latest upstream changes (presumably #145366) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot assigned pnkfelix Feb 8, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 8, 2024

This comment has been minimized.

Sign in to view

shepmaster mentioned this pull request Feb 8, 2024

Tracking Issue for enabling elided_lifetimes_in_paths lints #91639

Open

scottmcm reviewed Feb 8, 2024

View reviewed changes

compiler/rustc_lint/src/lib.rs Show resolved Hide resolved

scottmcm reviewed Feb 8, 2024

View reviewed changes

tests/ui/lifetimes/elided-lifetime-in-path-details.rs Outdated Show resolved Hide resolved

shepmaster force-pushed the split-elided-lifetimes-in-paths branch from eaf0446 to 8f5390c Compare February 9, 2024 19:40

This comment has been minimized.

Sign in to view

shepmaster force-pushed the split-elided-lifetimes-in-paths branch from 8f5390c to f1f5c32 Compare February 9, 2024 22:34

shepmaster commented Feb 10, 2024

View reviewed changes

compiler/rustc_lint_defs/src/builtin.rs Outdated Show resolved Hide resolved

shepmaster commented Feb 10, 2024

View reviewed changes

compiler/rustc_resolve/src/late.rs Outdated Show resolved Hide resolved

shepmaster commented Feb 10, 2024

View reviewed changes

tests/ui/lifetimes/elided-lifetime-in-path-details.rs Outdated Show resolved Hide resolved

shepmaster force-pushed the split-elided-lifetimes-in-paths branch from f1f5c32 to cc85718 Compare February 11, 2024 16:15

pnkfelix reviewed Feb 12, 2024

View reviewed changes

compiler/rustc_lint/src/lib.rs Outdated Show resolved Hide resolved

pnkfelix reviewed Feb 12, 2024

View reviewed changes

tests/ui/lifetimes/elided-lifetime-in-path-details.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 12, 2024

shepmaster force-pushed the split-elided-lifetimes-in-paths branch 2 times, most recently from f6d8513 to da16b9b Compare February 13, 2024 15:21

danielhenrymantilla reviewed Feb 28, 2024

View reviewed changes

compiler/rustc_lint/src/lib.rs Outdated Show resolved Hide resolved

danielhenrymantilla reviewed Feb 28, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

shepmaster force-pushed the split-elided-lifetimes-in-paths branch 2 times, most recently from 57a0a90 to 88dd6fc Compare March 11, 2024 20:18

jieyouxu mentioned this pull request May 30, 2025

Restore mismatched_lifetime_syntaxes wording about "elided lifetime has a name"? #141752

Closed

This comment has been minimized.

Sign in to view

This comment was marked as resolved.

Sign in to view

nnethercote reviewed Jun 15, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

jieyouxu reviewed Jun 19, 2025

View reviewed changes

cjgillot reviewed Jun 22, 2025

View reviewed changes

traviscross mentioned this pull request Jul 26, 2025

Interaction with mismatched-lifetime-syntaxes lint rust-lang/rust-analyzer#20304

Open

This comment has been minimized.

Sign in to view

shepmaster added 7 commits August 6, 2025 21:35

Test elided-lifetimes-in-paths outside of function signatures

615e373

Support trailing comma in add_lint_group!

f372f1f

Add a helper to rename lint groups

e9db3c7

Handle documenting a lint that was renamed to a group

96d0406

Extend HIR to track the source of a type

6a76e20

Add new HIR implementations of hidden lifetimes in paths lints

4e645f4

traviscross reviewed Aug 11, 2025

View reviewed changes

shepmaster mentioned this pull request Nov 10, 2025

Other errors suppress deny(mismatched_lifetime_syntaxes) #148705

Open

		lint_hidden_lifetime_in_path =
		paths containing hidden lifetime parameters are deprecated

		/// explicitly stated, even if it is the `'_` [placeholder
		/// lifetime].

	indicate the anonymous lifetime
	indicate the inferred lifetime

Uh oh!

Conversation

shepmaster commented Feb 8, 2024 • edited by jieyouxu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Background

History

Uh oh!

rustbot commented Feb 8, 2024

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

shepmaster commented Feb 9, 2024

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pnkfelix commented Feb 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pnkfelix commented Feb 12, 2024

Uh oh!

shepmaster commented Feb 12, 2024

Uh oh!

Uh oh!

danielhenrymantilla left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

This comment has been minimized.

This comment was marked as resolved.

shepmaster commented Jun 14, 2025

Uh oh!

rustbot commented Jun 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

jieyouxu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu commented Jun 19, 2025

Uh oh!

briansmith commented Jun 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Jun 24, 2025

Uh oh!

oli-obk commented Jul 25, 2025

Uh oh!

rustbot commented Aug 4, 2025

Uh oh!

This comment has been minimized.

rustbot commented Aug 4, 2025

Uh oh!

shepmaster commented Feb 8, 2024 •

edited by jieyouxu

Loading

pnkfelix commented Feb 12, 2024 •

edited

Loading