Deny todo!() in tidy#157706
Conversation
|
Some changes occurred in cc @BoxyUwU Some changes occurred in compiler/rustc_builtin_macros/src/autodiff.rs cc @ZuseZ4 Some changes occurred in compiler/rustc_sanitizers cc @rcvalle Some changes occurred in match checking cc @Nadrieril Some changes occurred to the CTFE machinery This PR changes rustc_public cc @oli-obk, @celinval, @ouz-a, @makai410 Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri |
|
r? @Kivooeo rustbot has assigned @Kivooeo. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
|
This PR changes a file inside Some changes occurred in coverage tests. cc @Zalathar Some changes occurred in GUI tests. Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @lolbinarycat The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease This PR modifies |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Ideally, this would be split into two different PRs:
|
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| impl<'a> Default for Directives<'a> { | ||
| fn default() -> Self { | ||
| Self { | ||
| $($name: Directive::default()),* | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This can be just #[derive(Default)]
| /// For drop-bomb behavior | ||
| Checked, |
There was a problem hiding this comment.
Can you expand on drop-bomb behavior here?
| /// `Ignore(false)` means that an `ignore-tidy-*` directive | ||
| /// has been provided, but is unnecessary. `Ignore(true)` | ||
| /// means that it is necessary (i.e. a warning would be | ||
| /// produced if `ignore-tidy-*` was not present). |
There was a problem hiding this comment.
This documentation is outdated
| impl<'a> Default for Directive<'a> { | ||
| fn default() -> Self { | ||
| Self::Deny | ||
| } | ||
| } |
There was a problem hiding this comment.
This can be #[derive(Default)]
| /// Some higher-level rule decides whether this directive is denied or ignored. | ||
| Derived(&'a Directive<'a>), |
There was a problem hiding this comment.
Q: Why is this necessary?
It looks like this is used to make the child set the used-ness in the parent... But since this is only relevant with Ignore it should just hold Rc<Cell<...>> imo.
| if trimmed.contains("ignore-tidy") { | ||
| return false; | ||
| } |
| let mut fake_directives = Directives::default(); | ||
| fake_directives.odd_backticks = directive; | ||
| fake_directives.check_usage(&mut check, file); |
There was a problem hiding this comment.
I honestly don't get what's going on here with fake_directives or why we need to store odd_backticks in comment_block...
| use run_make_support::{rfs, rustc}; | ||
|
|
||
| // This test make sure we don't get such following error: | ||
| // ignore-tidy-linelength |
There was a problem hiding this comment.
this one is useless? shouldn't tidy have caught it? 🤔
| //@ stderr-per-bitwidth | ||
| //@ ignore-endian-big | ||
| // ignore-tidy-linelength | ||
| //@ normalize-stderr: "╾─*ALLOC[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼" -> "╾ALLOC_ID$1╼" |
There was a problem hiding this comment.
why did you add this? doesn't seem like it changed .stderr...
|
Reminder, once the PR becomes ready for a review, use |
|
r? wafflelapkin |
|
|
View all comments
Implements rust-lang/compiler-team#999
Part of #158173