Let's #[expect] some lints: Stabilize lint_reasons (RFC 2383)

[xFrednet]

Let's give this another try! The previous stabilization attempt was stalled by some unresolved questions. These have been discussed in a lang team meeting. The last open question, regarding the semantics of the #[expect] attribute was decided on in #115980

I've just updated the stabilization report with the discussed questions and decisions. Luckily, the decision is inline with the current implementation.

This hopefully covers everything. Let's hope that the CI will be green like the spring.

fixes #115980
fixes #54503

---

For the release note author:

There are two Clippy lints which could be noteworthy:

• clippy::allow_attributes to restrict allow attributes in favor of #[expect] or to migrate #[allow] attributes to #[expect]
• clippy::allow_attributes_without_reason To require a reason for #[allow] attributes

Both are allow-by-default lints in the restriction category.

---

r? @wesleywiser

Tacking Issue: #54503
Stabilization Report: #54503 (comment)
Documentation Update: rust-lang/reference#1237

---

Roses are red,
Violets are blue,
Let's expect lints,
With reason clues

[xFrednet]

Alright, looks like most of the CI already agrees with my changes, let's kick off the reviewing process.

r? @wesleywiser

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

[oli-obk]

modulo the bootstrap cfgs in ui tests, the compiler side lgtm

[wesleywiser]

Implementation looks good to me! If I understand the current state correctly, T-lang has signed off on the previously unresolved questions but has not yet FCP'd stabilization. Is that correct?

I think the next step is then for us to start a stabilization FCP.

[xFrednet]

Implementation looks good to me! If I understand the current state correctly, T-lang has signed off on the previously unresolved questions but has not yet FCP'd stabilization. Is that correct?

Now that you say it, I believe this is correct. The previous stabilization report has been updated and should be sufficient. How do we kick off the FCP, do we ping T-lang in the tacking issue or here? #54503

And should I repost the updated stabilization report, just to have it at the bottom?

[wesleywiser]

Let's go ahead and do the FCP here, just so it's in the same PR that will eventually be merged 🙂

@rust-lang/lang, now that the implementation questions regarding #[expect] have been resolved, we feel the feature is ready for stabilization. @xFrednet has updated the stabilization report to reflect the current state.

[bors]

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

[tmandry]

Let's go ahead and do the FCP here, just so it's in the same PR that will eventually be merged 🙂

@rust-lang/lang, now that the implementation questions regarding #[expect] have been resolved, we feel the feature is ready for stabilization. @xFrednet has updated the stabilization report to reflect the current state.

Let's do it!

@rfcbot merge

[rfcbot]

Team member @tmandry has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @tmandry

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[rust-timer]

Finished benchmarking commit (4bc39f0): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.2%, 1.9%]	142
Regressions ❌ (secondary)	0.6%	[0.1%, 1.5%]	79
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.6%	[0.2%, 1.9%]	142

Max RSS (memory usage)

Results (primary -3.8%, secondary -3.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.8%	[-3.8%, -3.8%]	1
Improvements ✅ (secondary)	-3.8%	[-3.8%, -3.8%]	1
All ❌✅ (primary)	-3.8%	[-3.8%, -3.8%]	1

Cycles

Results (primary 1.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.4%	[0.8%, 2.3%]	21
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.4%	[0.8%, 2.3%]	21

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 695.122s -> 696.035s (0.13%)
Artifact size: 326.61 MiB -> 326.75 MiB (0.04%)

[jhpratt]

How could this have caused so many regressions? It's not as if everyone is using the attribute at this point in time.

[xFrednet]

It might be that the expectations were previously only collected/determined if the feature was enabled. That would be my guess, at least.

[Noratrieb]

it would be good to figure out the cause of the regression and whether it can be mitigated. if it's inherent to the feature, then we'll have to accept it

[qarmin]

One space too much after 0

[Noratrieb]

i checked the exa regression with cachegrind:

   3,586,341    <rustc_lint::levels::LintLevelsBuilder<rustc_lint::levels::QueryMapExpectationsWrapper>>::add_id
   1,119,278    <rustc_middle::lint::ShallowLintLevelMap>::lint_level_id_at_node
     676,462    rustc_hir::intravisit::walk_expr::<rustc_lint::levels::LintLevelsBuilder<rustc_lint::levels::QueryMapExpectationsWrapper>>

these are the functions that executed the most additional instructions and are therefore likely the cause of the regression.

[xFrednet]

This seems to be inline with the benchmark I did in #127217. There I disabled check_expectations which undid the regression of the 0.6%.

The additional calls mentioned above most likely come from the lint_expectations query.

The initial implementation of expect collected the lint level in the same pass that calculated the lint levels. This was refactored in #101620 and following PRs linked in that one to make this step incremental. (cc: @cjgillot, since you mostly did the refactoring AFAIK.)

For now, I would say that this regression is inherent to the feature, but it's very likely that it can be optimized further.

[pnkfelix]

Visiting for weekly rustc perf triage.

• wide collection of regressions.
• PR discussion indicates regression may be inherent to how #[expect] is implemented; it is also hypothesized to be "likely" that the implementation can be better optimized.
• not marking as triaged.

(my hope is that someone will look into the "further optimizations" that xFrednet alludes to above, and after we've done a reasonable amount of investigation there, then we can mark this as triaged.)

[myrrlyn]

As the (embarrassingly absent) RFC author, I am thrilled to see this land! Thank you all for your effort in actually taking my initial ideas and doing all the effort to refine, implement, and ship them. I can't wait to start using it.