Skip to content

Commit

Permalink
Support lint expectations for --force-warn lints (RFC 2383)
Browse files Browse the repository at this point in the history
  • Loading branch information
xFrednet committed Jun 16, 2022
1 parent ec55c61 commit 8527a3d
Show file tree
Hide file tree
Showing 19 changed files with 317 additions and 71 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ fn report_inline_asm(
}
let level = match level {
llvm::DiagnosticLevel::Error => Level::Error { lint: false },
llvm::DiagnosticLevel::Warning => Level::Warning,
llvm::DiagnosticLevel::Warning => Level::Warning(None),
llvm::DiagnosticLevel::Note | llvm::DiagnosticLevel::Remark => Level::Note,
};
cgcx.diag_emitter.inline_asm_error(cookie as u32, msg, level, source);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1761,7 +1761,7 @@ impl SharedEmitterMain {

let mut err = match level {
Level::Error { lint: false } => sess.struct_err(msg).forget_guarantee(),
Level::Warning => sess.struct_warn(msg),
Level::Warning(_) => sess.struct_warn(msg),
Level::Note => sess.struct_note_without_error(msg),
_ => bug!("Invalid inline asm diagnostic level"),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn annotation_type_for_level(level: Level) -> AnnotationType {
Level::Bug | Level::DelayedBug | Level::Fatal | Level::Error { .. } => {
AnnotationType::Error
}
Level::Warning => AnnotationType::Warning,
Level::Warning(_) => AnnotationType::Warning,
Level::Note | Level::OnceNote => AnnotationType::Note,
Level::Help => AnnotationType::Help,
// FIXME(#59346): Not sure how to map this level
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ impl Diagnostic {
| Level::Error { .. }
| Level::FailureNote => true,

Level::Warning
Level::Warning(_)
| Level::Note
| Level::OnceNote
| Level::Help
Expand All @@ -221,7 +221,9 @@ impl Diagnostic {
&mut self,
unstable_to_stable: &FxHashMap<LintExpectationId, LintExpectationId>,
) {
if let Level::Expect(expectation_id) = &mut self.level {
if let Level::Expect(expectation_id) | Level::Warning(Some(expectation_id)) =
&mut self.level
{
if expectation_id.is_stable() {
return;
}
Expand Down Expand Up @@ -445,7 +447,7 @@ impl Diagnostic {

/// Add a warning attached to this diagnostic.
pub fn warn(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self {
self.sub(Level::Warning, msg, MultiSpan::new(), None);
self.sub(Level::Warning(None), msg, MultiSpan::new(), None);
self
}

Expand All @@ -456,7 +458,7 @@ impl Diagnostic {
sp: S,
msg: impl Into<SubdiagnosticMessage>,
) -> &mut Self {
self.sub(Level::Warning, msg, sp.into(), None);
self.sub(Level::Warning(None), msg, sp.into(), None);
self
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl Emitter for JsonEmitter {
.into_iter()
.map(|mut diag| {
if diag.level == crate::Level::Allow {
diag.level = crate::Level::Warning;
diag.level = crate::Level::Warning(None);
}
FutureBreakageItem { diagnostic: Diagnostic::from_errors_diagnostic(&diag, self) }
})
Expand Down
96 changes: 64 additions & 32 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,23 @@ impl Handler {
result
}

/// Construct a builder at the `Warning` level at the given `span` and with the `msg`.
/// The `id` is used for lint emissions which should also fulfill a lint expectation.
///
/// Attempting to `.emit()` the builder will only emit if either:
/// * `can_emit_warnings` is `true`
/// * `is_force_warn` was set in `DiagnosticId::Lint`
pub fn struct_span_warn_with_expectation(
&self,
span: impl Into<MultiSpan>,
msg: impl Into<DiagnosticMessage>,
id: LintExpectationId,
) -> DiagnosticBuilder<'_, ()> {
let mut result = self.struct_warn_with_expectation(msg, id);
result.set_span(span);
result
}

/// Construct a builder at the `Allow` level at the given `span` and with the `msg`.
pub fn struct_span_allow(
&self,
Expand Down Expand Up @@ -688,7 +705,21 @@ impl Handler {
/// * `can_emit_warnings` is `true`
/// * `is_force_warn` was set in `DiagnosticId::Lint`
pub fn struct_warn(&self, msg: impl Into<DiagnosticMessage>) -> DiagnosticBuilder<'_, ()> {
DiagnosticBuilder::new(self, Level::Warning, msg)
DiagnosticBuilder::new(self, Level::Warning(None), msg)
}

/// Construct a builder at the `Warning` level with the `msg`. The `id` is used for
/// lint emissions which should also fulfill a lint expectation.
///
/// Attempting to `.emit()` the builder will only emit if either:
/// * `can_emit_warnings` is `true`
/// * `is_force_warn` was set in `DiagnosticId::Lint`
pub fn struct_warn_with_expectation(
&self,
msg: impl Into<DiagnosticMessage>,
id: LintExpectationId,
) -> DiagnosticBuilder<'_, ()> {
DiagnosticBuilder::new(self, Level::Warning(Some(id)), msg)
}

/// Construct a builder at the `Allow` level with the `msg`.
Expand Down Expand Up @@ -842,7 +873,7 @@ impl Handler {
}

pub fn span_warn(&self, span: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) {
self.emit_diag_at_span(Diagnostic::new(Warning, msg), span);
self.emit_diag_at_span(Diagnostic::new(Warning(None), msg), span);
}

pub fn span_warn_with_code(
Expand All @@ -851,7 +882,7 @@ impl Handler {
msg: impl Into<DiagnosticMessage>,
code: DiagnosticId,
) {
self.emit_diag_at_span(Diagnostic::new_with_code(Warning, Some(code), msg), span);
self.emit_diag_at_span(Diagnostic::new_with_code(Warning(None), Some(code), msg), span);
}

pub fn span_bug(&self, span: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) -> ! {
Expand Down Expand Up @@ -905,7 +936,7 @@ impl Handler {
}

pub fn warn(&self, msg: impl Into<DiagnosticMessage>) {
let mut db = DiagnosticBuilder::new(self, Warning, msg);
let mut db = DiagnosticBuilder::new(self, Warning(None), msg);
db.emit();
}

Expand Down Expand Up @@ -1010,13 +1041,10 @@ impl Handler {
for mut diag in diags.into_iter() {
diag.update_unstable_expectation_id(unstable_to_stable);

let stable_id = diag
.level
.get_expectation_id()
.expect("all diagnostics inside `unstable_expect_diagnostics` must have a `LintExpectationId`");
inner.fulfilled_expectations.insert(stable_id);

(*TRACK_DIAGNOSTICS)(&diag);
// Here the diagnostic is given back to `emit_diagnostic` where it was first
// intercepted. Now it should be processed as usual, since the unstable expectation
// id is now stable.
inner.emit_diagnostic(&mut diag);
}
}

Expand Down Expand Up @@ -1066,6 +1094,15 @@ impl HandlerInner {

// FIXME(eddyb) this should ideally take `diagnostic` by value.
fn emit_diagnostic(&mut self, diagnostic: &mut Diagnostic) -> Option<ErrorGuaranteed> {
// The `LintExpectationId` can be stable or unstable depending on when it was created.
// Diagnostics created before the definition of `HirId`s are unstable and can not yet
// be stored. Instead, they are buffered until the `LintExpectationId` is replaced by
// a stable one by the `LintLevelsBuilder`.
if let Some(LintExpectationId::Unstable { .. }) = diagnostic.level.get_expectation_id() {
self.unstable_expect_diagnostics.push(diagnostic.clone());
return None;
}

if diagnostic.level == Level::DelayedBug {
// FIXME(eddyb) this should check for `has_errors` and stop pushing
// once *any* errors were emitted (and truncate `delayed_span_bugs`
Expand All @@ -1082,7 +1119,12 @@ impl HandlerInner {
self.future_breakage_diagnostics.push(diagnostic.clone());
}

if diagnostic.level == Warning
if let Some(expectation_id) = diagnostic.level.get_expectation_id() {
self.suppressed_expected_diag = true;
self.fulfilled_expectations.insert(expectation_id);
}

if matches!(diagnostic.level, Warning(_))
&& !self.flags.can_emit_warnings
&& !diagnostic.is_force_warn()
{
Expand All @@ -1092,22 +1134,9 @@ impl HandlerInner {
return None;
}

// The `LintExpectationId` can be stable or unstable depending on when it was created.
// Diagnostics created before the definition of `HirId`s are unstable and can not yet
// be stored. Instead, they are buffered until the `LintExpectationId` is replaced by
// a stable one by the `LintLevelsBuilder`.
if let Level::Expect(LintExpectationId::Unstable { .. }) = diagnostic.level {
self.unstable_expect_diagnostics.push(diagnostic.clone());
return None;
}

(*TRACK_DIAGNOSTICS)(diagnostic);

if let Level::Expect(expectation_id) = diagnostic.level {
self.suppressed_expected_diag = true;
self.fulfilled_expectations.insert(expectation_id);
return None;
} else if diagnostic.level == Allow {
if matches!(diagnostic.level, Level::Expect(_) | Level::Allow) {
return None;
}

Expand Down Expand Up @@ -1144,7 +1173,7 @@ impl HandlerInner {
self.emitter.emit_diagnostic(&diagnostic);
if diagnostic.is_error() {
self.deduplicated_err_count += 1;
} else if diagnostic.level == Warning {
} else if let Warning(_) = diagnostic.level {
self.deduplicated_warn_count += 1;
}
}
Expand Down Expand Up @@ -1197,7 +1226,7 @@ impl HandlerInner {
match (errors.len(), warnings.len()) {
(0, 0) => return,
(0, _) => self.emitter.emit_diagnostic(&Diagnostic::new(
Level::Warning,
Level::Warning(None),
DiagnosticMessage::Str(warnings),
)),
(_, 0) => {
Expand Down Expand Up @@ -1430,7 +1459,10 @@ pub enum Level {
/// If this error comes from a lint, don't abort compilation even when abort_if_errors() is called.
lint: bool,
},
Warning,
/// This [`LintExpectationId`] is used for expected lint diagnostics, which should
/// also emit a warning due to the `force-warn` flag. In all other cases this should
/// be `None`.
Warning(Option<LintExpectationId>),
Note,
/// A note that is only emitted once.
OnceNote,
Expand All @@ -1453,7 +1485,7 @@ impl Level {
Bug | DelayedBug | Fatal | Error { .. } => {
spec.set_fg(Some(Color::Red)).set_intense(true);
}
Warning => {
Warning(_) => {
spec.set_fg(Some(Color::Yellow)).set_intense(cfg!(windows));
}
Note | OnceNote => {
Expand All @@ -1472,7 +1504,7 @@ impl Level {
match self {
Bug | DelayedBug => "error: internal compiler error",
Fatal | Error { .. } => "error",
Warning => "warning",
Warning(_) => "warning",
Note | OnceNote => "note",
Help => "help",
FailureNote => "failure-note",
Expand All @@ -1487,7 +1519,7 @@ impl Level {

pub fn get_expectation_id(&self) -> Option<LintExpectationId> {
match self {
Level::Expect(id) => Some(*id),
Level::Expect(id) | Level::Warning(Some(id)) => Some(*id),
_ => None,
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/proc_macro_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl ToInternal<rustc_errors::Level> for Level {
fn to_internal(self) -> rustc_errors::Level {
match self {
Level::Error => rustc_errors::Level::Error { lint: false },
Level::Warning => rustc_errors::Level::Warning,
Level::Warning => rustc_errors::Level::Warning(None),
Level::Note => rustc_errors::Level::Note,
Level::Help => rustc_errors::Level::Help,
_ => unreachable!("unknown proc_macro::Level variant: {:?}", self),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ impl LintStore {
registered_tools: &RegisteredTools,
) {
let (tool_name, lint_name_only) = parse_lint_and_tool_name(lint_name);
if lint_name_only == crate::WARNINGS.name_lower() && level == Level::ForceWarn {
if lint_name_only == crate::WARNINGS.name_lower() && matches!(level, Level::ForceWarn(_)) {
struct_span_err!(
sess,
DUMMY_SP,
Expand Down Expand Up @@ -375,7 +375,7 @@ impl LintStore {
match level {
Level::Allow => "-A",
Level::Warn => "-W",
Level::ForceWarn => "--force-warn",
Level::ForceWarn(_) => "--force-warn",
Level::Deny => "-D",
Level::Forbid => "-F",
Level::Expect(_) => {
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_lint/src/expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ fn check_expectations(tcx: TyCtxt<'_>, tool_filter: Option<Symbol>) {
let lint_expectations = &tcx.lint_levels(()).lint_expectations;

for (id, expectation) in lint_expectations {
if !fulfilled_expectations.contains(id)
&& tool_filter.map_or(true, |filter| expectation.lint_tool == Some(filter))
{
// This check will always be true, since `lint_expectations` only
// holds stable ids
if let LintExpectationId::Stable { hir_id, .. } = id {
// This check will always be true, since `lint_expectations` only
// holds stable ids
if let LintExpectationId::Stable { hir_id, .. } = id {
if !fulfilled_expectations.contains(&id)
&& tool_filter.map_or(true, |filter| expectation.lint_tool == Some(filter))
{
emit_unfulfilled_expectation_lint(tcx, *hir_id, expectation);
} else {
unreachable!("at this stage all `LintExpectationId`s are stable");
}
} else {
unreachable!("at this stage all `LintExpectationId`s are stable");
}
}
}
Expand Down
22 changes: 16 additions & 6 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ impl<'s> LintLevelsBuilder<'s> {
};
for id in ids {
// ForceWarn and Forbid cannot be overridden
if let Some((Level::ForceWarn | Level::Forbid, _)) = self.current_specs().get(&id) {
if let Some((Level::ForceWarn(_) | Level::Forbid, _)) =
self.current_specs().get(&id)
{
continue;
}

Expand Down Expand Up @@ -226,11 +228,18 @@ impl<'s> LintLevelsBuilder<'s> {
return;
}

if let Level::ForceWarn = old_level {
self.current_specs_mut().insert(id, (old_level, old_src));
} else {
self.current_specs_mut().insert(id, (level, src));
}
match (old_level, level) {
// If the new level is an expectation store it in `ForceWarn`
(Level::ForceWarn(_), Level::Expect(expectation_id)) => self
.current_specs_mut()
.insert(id, (Level::ForceWarn(Some(expectation_id)), old_src)),
// Keep `ForceWarn` level but drop the expectation
(Level::ForceWarn(_), _) => {
self.current_specs_mut().insert(id, (Level::ForceWarn(None), old_src))
}
// Set the lint level as normal
_ => self.current_specs_mut().insert(id, (level, src)),
};
}

/// Pushes a list of AST lint attributes onto this context.
Expand Down Expand Up @@ -269,6 +278,7 @@ impl<'s> LintLevelsBuilder<'s> {

let level = match Level::from_attr(attr) {
None => continue,
// This is the only lint level with a `LintExpectationId` that can be created from an attribute
Some(Level::Expect(unstable_id)) if let Some(hir_id) = source_hir_id => {
let stable_id = self.create_stable_id(unstable_id, hir_id, attr_index);

Expand Down
Loading

0 comments on commit 8527a3d

Please sign in to comment.