Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lints_that_dont_need_to_run: never skip future-compat-reported lints #133108

Merged
merged 1 commit into from
Nov 20, 2024
+27 −16
Merged

lints_that_dont_need_to_run: never skip future-compat-reported lints #133108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions compiler/rustc_lint/src/levels.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,17 +123,19 @@ fn lints_that_dont_need_to_run(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LintId> {
let dont_need_to_run: FxIndexSet<LintId> = store
.get_lints()
.into_iter()
.filter(|lint| {
// Lints that show up in future-compat reports must always be run.
let has_future_breakage =
lint.future_incompatible.is_some_and(|fut| fut.reason.has_future_breakage());
!has_future_breakage && !lint.eval_always
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to have these two as different methods?

Copy link
Member Author

@RalfJung RalfJung Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's easier to first do the pure filtering, and then the filter_map part that also transforms the data. This also reduces rightward drift in the filter_map closure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could the fitire breakage check be moved earlier? i.e. automatically set eval_always for all future breakage lints

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would require putting the logic for this into the declare_lint macro. IMO that's a bad idea, I'd rather not have this logic hidden in a macro.

.filter_map(|lint| {
if !lint.eval_always {
let lint_level = map.lint_level_id_at_node(tcx, LintId::of(lint), CRATE_HIR_ID);
if matches!(lint_level, (Level::Allow, ..))
|| (matches!(lint_level, (.., LintLevelSource::Default)))
&& lint.default_level(tcx.sess.edition()) == Level::Allow
{
Some(LintId::of(lint))
} else {
None
}
let lint_level = map.lint_level_id_at_node(tcx, LintId::of(lint), CRATE_HIR_ID);
if matches!(lint_level, (Level::Allow, ..))
|| (matches!(lint_level, (.., LintLevelSource::Default)))
&& lint.default_level(tcx.sess.edition()) == Level::Allow
{
Some(LintId::of(lint))
} else {
None
}
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,20 @@ impl FutureIncompatibilityReason {
| FutureIncompatibilityReason::Custom(_) => None,
}
}

pub fn has_future_breakage(self) -> bool {
match self {
FutureIncompatibilityReason::FutureReleaseErrorReportInDeps => true,

FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps
| FutureIncompatibilityReason::FutureReleaseSemanticsChange
| FutureIncompatibilityReason::EditionError(_)
| FutureIncompatibilityReason::EditionSemanticsChange(_)
| FutureIncompatibilityReason::EditionAndFutureReleaseError(_)
| FutureIncompatibilityReason::EditionAndFutureReleaseSemanticsChange(_)
| FutureIncompatibilityReason::Custom(_) => false,
}
}
}

impl FutureIncompatibleInfo {
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_middle/src/lint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,7 @@ pub fn lint_level(
let has_future_breakage = future_incompatible.map_or(
// Default allow lints trigger too often for testing.
sess.opts.unstable_opts.future_incompat_test && lint.default_level != Level::Allow,
|incompat| {
matches!(
incompat.reason,
FutureIncompatibilityReason::FutureReleaseErrorReportInDeps
)
},
|incompat| incompat.reason.has_future_breakage(),
);

// Convert lint level to error level.
Expand Down
Loading