Add an allow attribute suggestion along with the implied by suggestion #114089

Urgau · 2023-07-26T08:59:04Z

This PR adds an #[allow(...)] attribute hep suggestion along with the implied by suggestion:

  note: `-W dead-code` implied by `-W unused`
+ help: to override `-W unused` add `#[allow(dead_code)]`

This PR also adds the OnceHelp lint level (similar to OnceNote) to only put the help message one time, like the implied note.

Related to #114030

rustbot · 2023-07-26T08:59:09Z

r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-07-26T10:10:48Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler/rustc_middle/src/lint.rs

xFrednet · 2023-07-26T11:37:57Z

Do we maybe want to suggest #[expect] instead, if the lint_reasons feature is enabled? See RFC 2383

This is just a biased suggestion since I've implemented that attribute and am currently trying to push the stabilization, so please ignore it if you don't agree.

Besides that suggestion, I really like this change 👍

ojeda · 2023-07-26T14:38:15Z

@xFrednet That sounds sensible and may be a better default than allow, though it may not always be the desired one.

For instance, in the "temporary/development allow" case, consider dead_code. One may be still experimenting with a few things, and thus calling a function or commenting the call from time to time. So the code may go between the dead and used code a few times. In this case, having the allow may be more pleasant. But others may still prefer to have to use expect, in order to avoid forgetting about it even if it requires a bit more work.

In the "committed/permanent allow" case, it is definitely nice to know when something is superfluous. Having said that, I wonder about supporting several toolchain versions. If the behavior of a lint changes, then one could have an expect that starts triggering in a new release, in which case they would need to change it to allow. This may be undesirable for projects that aim to be warning-free as much as possible. I guess it is not much of a concern for rustc lints, assuming they are more "stable" (in behavior) vs. Clippy ones or other custom ones a project may use.

Perhaps the best solution is to suggest both, and let users decide.

By the way, it may be even better/easier to already go for expect if the lint_reasons feature would be split in two features (in the tracking issue I see you said you would prefer not to, though).

compiler/rustc_errors/src/lib.rs

compiler/rustc_middle/src/lint.rs

Urgau · 2023-08-24T11:42:00Z

@wesleywiser still interested in reviewing this MR ?

Urgau · 2023-09-02T09:42:17Z

r? compiler

src/tools/clippy/tests/ui-toml/absolute_paths/absolute_paths.allow_crates.stderr

bors · 2023-09-04T16:25:11Z

💔 Test failed - checks-actions

ehuss · 2023-09-04T16:25:48Z

@bors retry

error decoding response body: operation timed out

Still investigating.

…rochenkov Add an allow attribute suggestion along with the implied by suggestion This PR adds an `#[allow(...)]` attribute hep suggestion along with the implied by suggestion: ```diff note: `-W dead-code` implied by `-W unused` + help: to override `-W unused` add `#[allow(dead_code)]` ``` This PR also adds the `OnceHelp` lint level (similar to `OnceNote`) to only put the help message one time, like the implied note. Related to rust-lang#114030

rust-log-analyzer · 2023-09-04T16:27:12Z

The job dist-x86_64-msvc failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    0: error decoding response body: operation timed out
    1: operation timed out

Stack backtrace:
   0: backtrace::backtrace::dbghelp::trace
             at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\backtrace-0.3.69\src\backtrace\dbghelp.rs:98
             at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\backtrace-0.3.69\src\backtrace\mod.rs:66
             at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\backtrace-0.3.69\src\backtrace\mod.rs:66
   2: backtrace::backtrace::trace<anyhow::backtrace::capture::impl$4::create::closure_env$0>
             at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\backtrace-0.3.69\src\backtrace\mod.rs:53
   3: anyhow::backtrace::capture::Backtrace::create
             at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\anyhow-1.0.75\src\backtrace.rs:216
   4: anyhow::backtrace::capture::Backtrace::capture
             at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\anyhow-1.0.75\src\backtrace.rs:204
   5: anyhow::error::impl$1::from<reqwest::error::Error>
             at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\anyhow-1.0.75\src\error.rs:551
   6: opt_dist::environment::windows::impl$1::prepare_rustc_perf
             at /rustc/1119c18100c1d15eeb2b3e6af5356c54ff602c30/src\tools\opt-dist\src\environment\windows.rs:46
   7: opt_dist::execute_pipeline::closure$0
             at /rustc/1119c18100c1d15eeb2b3e6af5356c54ff602c30/src\tools\opt-dist\src\main.rs:37
   8: opt_dist::utils::with_log_group<opt_dist::execute_pipeline::closure_env$0,enum2$<core::result::Result<tuple$<>,anyhow::Error> > >
             at /rustc/1119c18100c1d15eeb2b3e6af5356c54ff602c30/src\tools\opt-dist\src\utils\mod.rs:65
             at /rustc/1119c18100c1d15eeb2b3e6af5356c54ff602c30/src\tools\opt-dist\src\main.rs:207
  10: core::ops::function::FnOnce::call_once
             at /rustc/680cdf8168a906b4ea80af673c64e4a16f77be57\library\core\src\ops\function.rs:250
             at /rustc/680cdf8168a906b4ea80af673c64e4a16f77be57\library\core\src\ops\function.rs:250
  11: std::sys_common::backtrace::__rust_begin_short_backtrace<enum2$<core::result::Result<tuple$<>,anyhow::Error> > (*)(),enum2$<core::result::Result<tuple$<>,anyhow::Error> > >
             at /rustc/680cdf8168a906b4ea80af673c64e4a16f77be57\library\std\src\sys_common\backtrace.rs:154
             at /rustc/680cdf8168a906b4ea80af673c64e4a16f77be57\library\std\src\rt.rs:166
  13: core::ops::function::FnOnce::call_once
             at /rustc/680cdf8168a906b4ea80af673c64e4a16f77be57\library\core\src\ops\function.rs:250
             at /rustc/680cdf8168a906b4ea80af673c64e4a16f77be57\library\core\src\ops\function.rs:250
  14: core::ops::function::FnOnce::call_once<std::rt::lang_start::closure_env$0<enum2$<core::result::Result<tuple$<>,anyhow::Error> > >,tuple$<> >
             at /rustc/680cdf8168a906b4ea80af673c64e4a16f77be57\library\std\src\rt.rs:166
             at /rustc/680cdf8168a906b4ea80af673c64e4a16f77be57/library\std\src\rt.rs:148
  16: std::panicking::try::do_call
             at /rustc/680cdf8168a906b4ea80af673c64e4a16f77be57/library\std\src\panicking.rs:524
  17: std::panicking::try

bors · 2023-09-04T21:18:42Z

⌛ Testing commit 9190e96 with merge dbbc2d0cfbf7328b301488f385be5df4efdfcb66...

bors · 2023-09-04T21:19:15Z

💔 Test failed - checks-actions

rust-log-analyzer · 2023-09-04T21:28:51Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Prepare workflow directory
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v3' (SHA:f43a0e5ff2bd294095638e18286ca9a3d1956744)
##[error]Can't use 'tar -xzf' extract archive file: /home/runner/work/_actions/_temp_7af74abc-37f2-4759-9ff4-e6004d6e6700/d3c29ed9-89cc-4599-ab90-b3bc137ce4d5.tar.gz. return code: 2.

ehuss · 2023-09-04T21:30:49Z

@bors retry

GitHub checkout failed

ehuss · 2023-09-04T23:09:51Z

@bors p=101

bors · 2023-09-04T23:10:06Z

⌛ Testing commit 9190e96 with merge 04374cd...

bors · 2023-09-05T00:51:08Z

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 04374cd to master...

rust-timer · 2023-09-05T03:13:36Z

Finished benchmarking commit (04374cd): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

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)	2.2%	[2.2%, 2.2%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-1.4%	[-1.4%, -1.4%]	1
All ❌✅ (primary)	1.3%	[-0.5%, 2.2%]	3

Cycles

Results

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)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-3.6%, -3.4%]	2
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 629.118s -> 629.115s (-0.00%)
Artifact size: 316.16 MiB -> 316.11 MiB (-0.01%)

carols10cents · 2023-09-12T17:19:55Z

I am SO excited by this, you have no idea. Not having something to easily copy-paste, and needing to change hyphens to underscores, is one of the most annoying papercuts I hit with Clippy.

@Urgau know that I will be sending thoughts of thanks your way for YEARS to come every time I can copy-paste an allow from this message ❤️ ❤️ ❤️ ❤️ ❤️

rustbot assigned wesleywiser Jul 26, 2023

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 Jul 26, 2023

This comment has been minimized.

Sign in to view

ojeda mentioned this pull request Jul 26, 2023

rustc wanted features & bugfixes Rust-for-Linux/linux#355

Open

51 tasks

Urgau force-pushed the allow-with-implied-by branch from 0bc470c to 9e18748 Compare July 26, 2023 09:24

blyxyas reviewed Jul 26, 2023

View reviewed changes

compiler/rustc_middle/src/lint.rs Outdated Show resolved Hide resolved

Urgau force-pushed the allow-with-implied-by branch from b1af763 to a7a6a1b Compare July 27, 2023 16:25

This comment was marked as resolved.

Sign in to view

Urgau force-pushed the allow-with-implied-by branch from a7a6a1b to 2c529d1 Compare August 1, 2023 12:09

wesleywiser reviewed Aug 1, 2023

View reviewed changes

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

compiler/rustc_middle/src/lint.rs Outdated Show resolved Hide resolved

Urgau force-pushed the allow-with-implied-by branch 2 times, most recently from 6643dfb to 1b4e330 Compare August 3, 2023 16:44

This comment has been minimized.

Sign in to view

Urgau force-pushed the allow-with-implied-by branch from 1b4e330 to 61be1dc Compare August 3, 2023 16:51

This comment was marked as resolved.

Sign in to view

Urgau force-pushed the allow-with-implied-by branch from 61be1dc to 64e7544 Compare August 11, 2023 19:20

Urgau force-pushed the allow-with-implied-by branch from 64e7544 to e796835 Compare August 24, 2023 11:41

rustbot assigned petrochenkov and unassigned wesleywiser Sep 2, 2023

petrochenkov reviewed Sep 4, 2023

View reviewed changes

src/tools/clippy/tests/ui-toml/absolute_paths/absolute_paths.allow_crates.stderr Outdated Show resolved Hide resolved

petrochenkov 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 Sep 4, 2023

Add OnceHelp lint level (same as OnceNote, except for help)

eba5b44

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 4, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2023

matthiaskrgr mentioned this pull request Sep 4, 2023

Rollup of 7 pull requests #115537

Closed

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 4, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 5, 2023

bors merged commit 04374cd into rust-lang:master Sep 5, 2023
11 checks passed

rustbot added this to the 1.74.0 milestone Sep 5, 2023

Urgau deleted the allow-with-implied-by branch September 5, 2023 07:53

ojeda mentioned this pull request Sep 5, 2023

Diagnostics for -D flags make it slightly harder to allow vs. #![deny(...)] #114030

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an allow attribute suggestion along with the implied by suggestion #114089

Add an allow attribute suggestion along with the implied by suggestion #114089

Urgau commented Jul 26, 2023 •

edited

Loading

rustbot commented Jul 26, 2023

This comment has been minimized.

rustbot commented Jul 26, 2023

xFrednet commented Jul 26, 2023 •

edited

Loading

ojeda commented Jul 26, 2023

This comment was marked as resolved.

This comment has been minimized.

This comment was marked as resolved.

Urgau commented Aug 24, 2023

Urgau commented Sep 2, 2023

bors commented Sep 4, 2023

ehuss commented Sep 4, 2023

rust-log-analyzer commented Sep 4, 2023

bors commented Sep 4, 2023

bors commented Sep 4, 2023

rust-log-analyzer commented Sep 4, 2023

ehuss commented Sep 4, 2023

ehuss commented Sep 4, 2023

bors commented Sep 4, 2023

bors commented Sep 5, 2023

rust-timer commented Sep 5, 2023

carols10cents commented Sep 12, 2023

Add an allow attribute suggestion along with the implied by suggestion #114089

Add an allow attribute suggestion along with the implied by suggestion #114089

Conversation

Urgau commented Jul 26, 2023 • edited Loading

rustbot commented Jul 26, 2023

This comment has been minimized.

rustbot commented Jul 26, 2023

xFrednet commented Jul 26, 2023 • edited Loading

ojeda commented Jul 26, 2023

This comment was marked as resolved.

This comment has been minimized.

This comment was marked as resolved.

Urgau commented Aug 24, 2023

Urgau commented Sep 2, 2023

bors commented Sep 4, 2023

ehuss commented Sep 4, 2023

rust-log-analyzer commented Sep 4, 2023

bors commented Sep 4, 2023

bors commented Sep 4, 2023

rust-log-analyzer commented Sep 4, 2023

ehuss commented Sep 4, 2023

ehuss commented Sep 4, 2023

bors commented Sep 4, 2023

bors commented Sep 5, 2023

rust-timer commented Sep 5, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

carols10cents commented Sep 12, 2023

Urgau commented Jul 26, 2023 •

edited

Loading

xFrednet commented Jul 26, 2023 •

edited

Loading