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.

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

Cleanup things in and around Diagnostic #119763

Merged
merged 6 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ fn not_testable_error(cx: &ExtCtxt<'_>, attr_sp: Span, item: Option<&ast::Item>)
let level = match item.map(|i| &i.kind) {
// These were a warning before #92959 and need to continue being that to avoid breaking
// stable user code (#94508).
Some(ast::ItemKind::MacCall(_)) => Level::Warning(None),
Some(ast::ItemKind::MacCall(_)) => Level::Warning,
_ => Level::Error,
};
let mut err = DiagnosticBuilder::<()>::new(dcx, level, msg);
Expand Down
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 @@ -417,7 +417,7 @@ fn report_inline_asm(
}
let level = match level {
llvm::DiagnosticLevel::Error => Level::Error,
llvm::DiagnosticLevel::Warning => Level::Warning(None),
llvm::DiagnosticLevel::Warning => Level::Warning,
llvm::DiagnosticLevel::Note | llvm::DiagnosticLevel::Remark => Level::Note,
};
cgcx.diag_emitter.inline_asm_error(cookie as u32, msg, level, source);
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1847,14 +1847,9 @@ impl SharedEmitterMain {
dcx.emit_diagnostic(d);
}
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
let err_level = match level {
Level::Error => Level::Error,
Level::Warning(_) => Level::Warning(None),
Level::Note => Level::Note,
_ => bug!("Invalid inline asm diagnostic level"),
};
assert!(matches!(level, Level::Error | Level::Warning | Level::Note));
let msg = msg.strip_prefix("error: ").unwrap_or(&msg).to_string();
let mut err = DiagnosticBuilder::<()>::new(sess.dcx(), err_level, msg);
let mut err = DiagnosticBuilder::<()>::new(sess.dcx(), level, msg);

// If the cookie is 0 then we don't have span information.
if cookie != 0 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn source_string(file: Lrc<SourceFile>, line: &Line) -> String {
fn annotation_type_for_level(level: Level) -> AnnotationType {
match level {
Level::Bug | Level::DelayedBug | Level::Fatal | Level::Error => AnnotationType::Error,
Level::Warning(_) => AnnotationType::Warning,
Level::ForceWarning(_) | Level::Warning => AnnotationType::Warning,
Level::Note | Level::OnceNote => AnnotationType::Note,
Level::Help | Level::OnceHelp => AnnotationType::Help,
// FIXME(#59346): Not sure how to map this level
Expand Down
17 changes: 10 additions & 7 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ pub enum DiagnosticId {
name: String,
/// Indicates whether this lint should show up in cargo's future breakage report.
has_future_breakage: bool,
is_force_warn: bool,
},
}

Expand Down Expand Up @@ -248,7 +247,8 @@ impl Diagnostic {
true
}

Level::Warning(_)
Level::ForceWarning(_)
| Level::Warning
| Level::Note
| Level::OnceNote
| Level::Help
Expand All @@ -262,7 +262,7 @@ impl Diagnostic {
&mut self,
unstable_to_stable: &FxIndexMap<LintExpectationId, LintExpectationId>,
) {
if let Level::Expect(expectation_id) | Level::Warning(Some(expectation_id)) =
if let Level::Expect(expectation_id) | Level::ForceWarning(Some(expectation_id)) =
&mut self.level
{
if expectation_id.is_stable() {
Expand Down Expand Up @@ -292,8 +292,11 @@ impl Diagnostic {
}

pub(crate) fn is_force_warn(&self) -> bool {
match self.code {
Some(DiagnosticId::Lint { is_force_warn, .. }) => is_force_warn,
match self.level {
Level::ForceWarning(_) => {
assert!(self.is_lint);
true
}
_ => false,
}
}
Expand Down Expand Up @@ -472,7 +475,7 @@ impl Diagnostic {
/// Add a warning attached to this diagnostic.
#[rustc_lint_diagnostics]
pub fn warn(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self {
self.sub(Level::Warning(None), msg, MultiSpan::new());
self.sub(Level::Warning, msg, MultiSpan::new());
self
}

Expand All @@ -484,7 +487,7 @@ impl Diagnostic {
sp: S,
msg: impl Into<SubdiagnosticMessage>,
) -> &mut Self {
self.sub(Level::Warning(None), msg, sp.into());
self.sub(Level::Warning, msg, sp.into());
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 @@ -198,7 +198,7 @@ impl Emitter for JsonEmitter {
.into_iter()
.map(|mut diag| {
if diag.level == crate::Level::Allow {
diag.level = crate::Level::Warning(None);
diag.level = crate::Level::Warning;
}
FutureBreakageItem {
diagnostic: EmitTyped::Diagnostic(Diagnostic::from_errors_diagnostic(
Expand Down
97 changes: 41 additions & 56 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,15 +420,21 @@ pub struct DiagCtxt {
/// as well as inconsistent state observation.
struct DiagCtxtInner {
flags: DiagCtxtFlags,

/// The number of lint errors that have been emitted.
lint_err_count: usize,
/// The number of errors that have been emitted, including duplicates.
///
/// This is not necessarily the count that's reported to the user once
/// compilation ends.
err_count: usize,
warn_count: usize,
deduplicated_err_count: usize,
/// The warning count, used for a recap upon finishing
deduplicated_warn_count: usize,
/// Has this diagnostic context printed any diagnostics? (I.e. has
/// `self.emitter.emit_diagnostic()` been called?
has_printed: bool,

emitter: Box<DynEmitter>,
span_delayed_bugs: Vec<DelayedDiagnostic>,
good_path_delayed_bugs: Vec<DelayedDiagnostic>,
Expand All @@ -455,9 +461,6 @@ struct DiagCtxtInner {
/// When `.abort_if_errors()` is called, these are also emitted.
stashed_diagnostics: FxIndexMap<(Span, StashKey), Diagnostic>,

/// The warning count, used for a recap upon finishing
deduplicated_warn_count: usize,

future_breakage_diagnostics: Vec<Diagnostic>,

/// The [`Self::unstable_expect_diagnostics`] should be empty when this struct is
Expand Down Expand Up @@ -513,7 +516,7 @@ fn default_track_diagnostic(diag: Diagnostic, f: &mut dyn FnMut(Diagnostic)) {
(*f)(diag)
}

pub static TRACK_DIAGNOSTICS: AtomicRef<fn(Diagnostic, &mut dyn FnMut(Diagnostic))> =
pub static TRACK_DIAGNOSTIC: AtomicRef<fn(Diagnostic, &mut dyn FnMut(Diagnostic))> =
AtomicRef::new(&(default_track_diagnostic as _));

#[derive(Copy, Clone, Default)]
Expand Down Expand Up @@ -547,8 +550,7 @@ impl Drop for DiagCtxtInner {
// instead of "require some error happened". Sadly that isn't ideal, as
// lints can be `#[allow]`'d, potentially leading to this triggering.
// Also, "good path" should be replaced with a better naming.
let has_any_message = self.err_count > 0 || self.lint_err_count > 0 || self.warn_count > 0;
if !has_any_message && !self.suppressed_expected_diag && !std::thread::panicking() {
if !self.has_printed && !self.suppressed_expected_diag && !std::thread::panicking() {
let bugs = std::mem::replace(&mut self.good_path_delayed_bugs, Vec::new());
self.flush_delayed(
bugs,
Expand Down Expand Up @@ -594,9 +596,9 @@ impl DiagCtxt {
flags: DiagCtxtFlags { can_emit_warnings: true, ..Default::default() },
lint_err_count: 0,
err_count: 0,
warn_count: 0,
deduplicated_err_count: 0,
deduplicated_warn_count: 0,
has_printed: false,
emitter,
span_delayed_bugs: Vec::new(),
good_path_delayed_bugs: Vec::new(),
Expand Down Expand Up @@ -647,10 +649,11 @@ impl DiagCtxt {
/// the overall count of emitted error diagnostics.
pub fn reset_err_count(&self) {
let mut inner = self.inner.borrow_mut();
inner.lint_err_count = 0;
inner.err_count = 0;
inner.warn_count = 0;
inner.deduplicated_err_count = 0;
inner.deduplicated_warn_count = 0;
inner.has_printed = false;

// actually free the underlying memory (which `clear` would not do)
inner.span_delayed_bugs = Default::default();
Expand All @@ -669,16 +672,11 @@ impl DiagCtxt {
let key = (span.with_parent(None), key);

if diag.is_error() {
if diag.level == Error && diag.is_lint {
if diag.is_lint {
inner.lint_err_count += 1;
} else {
inner.err_count += 1;
}
} else {
// Warnings are only automatically flushed if they're forced.
if diag.is_force_warn() {
inner.warn_count += 1;
}
}

// FIXME(Centril, #69537): Consider reintroducing panic on overwriting a stashed diagnostic
Expand All @@ -693,15 +691,11 @@ impl DiagCtxt {
let key = (span.with_parent(None), key);
let diag = inner.stashed_diagnostics.remove(&key)?;
if diag.is_error() {
if diag.level == Error && diag.is_lint {
if diag.is_lint {
inner.lint_err_count -= 1;
} else {
inner.err_count -= 1;
}
} else {
if diag.is_force_warn() {
inner.warn_count -= 1;
}
}
Some(DiagnosticBuilder::new_diagnostic(self, diag))
}
Expand Down Expand Up @@ -738,7 +732,7 @@ impl DiagCtxt {
#[rustc_lint_diagnostics]
#[track_caller]
pub fn struct_warn(&self, msg: impl Into<DiagnosticMessage>) -> DiagnosticBuilder<'_, ()> {
DiagnosticBuilder::new(self, Warning(None), msg)
DiagnosticBuilder::new(self, Warning, msg)
}

/// Construct a builder at the `Allow` level with the `msg`.
Expand Down Expand Up @@ -1005,7 +999,7 @@ impl DiagCtxt {
(0, 0) => return,
(0, _) => inner
.emitter
.emit_diagnostic(&Diagnostic::new(Warning(None), DiagnosticMessage::Str(warnings))),
.emit_diagnostic(&Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))),
(_, 0) => {
inner.emit_diagnostic(Diagnostic::new(Fatal, errors));
}
Expand Down Expand Up @@ -1094,7 +1088,7 @@ impl DiagCtxt {
&'a self,
warning: impl IntoDiagnostic<'a, ()>,
) -> DiagnosticBuilder<'a, ()> {
warning.into_diagnostic(self, Warning(None))
warning.into_diagnostic(self, Warning)
}

#[track_caller]
Expand Down Expand Up @@ -1241,21 +1235,17 @@ impl DiagCtxtInner {
for diag in diags {
// Decrement the count tracking the stash; emitting will increment it.
if diag.is_error() {
if diag.level == Error && diag.is_lint {
if diag.is_lint {
self.lint_err_count -= 1;
} else {
self.err_count -= 1;
}
} else {
if diag.is_force_warn() {
self.warn_count -= 1;
} else {
// Unless they're forced, don't flush stashed warnings when
// there are errors, to avoid causing warning overload. The
// stash would've been stolen already if it were important.
if has_errors {
continue;
}
// Unless they're forced, don't flush stashed warnings when
// there are errors, to avoid causing warning overload. The
// stash would've been stolen already if it were important.
if !diag.is_force_warn() && has_errors {
continue;
}
}
let reported_this = self.emit_diagnostic(diag);
Expand Down Expand Up @@ -1304,23 +1294,20 @@ impl DiagCtxtInner {
self.fulfilled_expectations.insert(expectation_id.normalize());
}

if matches!(diagnostic.level, Warning(_))
&& !self.flags.can_emit_warnings
&& !diagnostic.is_force_warn()
{
if diagnostic.level == Warning && !self.flags.can_emit_warnings {
if diagnostic.has_future_breakage() {
(*TRACK_DIAGNOSTICS)(diagnostic, &mut |_| {});
(*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {});
}
return None;
}

if matches!(diagnostic.level, Expect(_) | Allow) {
(*TRACK_DIAGNOSTICS)(diagnostic, &mut |_| {});
(*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {});
return None;
}

let mut guaranteed = None;
(*TRACK_DIAGNOSTICS)(diagnostic, &mut |mut diagnostic| {
(*TRACK_DIAGNOSTIC)(diagnostic, &mut |mut diagnostic| {
if let Some(ref code) = diagnostic.code {
self.emitted_diagnostic_codes.insert(code.clone());
}
Expand Down Expand Up @@ -1359,12 +1346,13 @@ impl DiagCtxtInner {
self.emitter.emit_diagnostic(&diagnostic);
if diagnostic.is_error() {
self.deduplicated_err_count += 1;
} else if let Warning(_) = diagnostic.level {
} else if matches!(diagnostic.level, ForceWarning(_) | Warning) {
self.deduplicated_warn_count += 1;
}
self.has_printed = true;
}
if diagnostic.is_error() {
if diagnostic.level == Error && diagnostic.is_lint {
if diagnostic.is_lint {
self.bump_lint_err_count();
} else {
self.bump_err_count();
Expand All @@ -1374,8 +1362,6 @@ impl DiagCtxtInner {
{
guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted());
}
} else {
self.bump_warn_count();
}
});

Expand Down Expand Up @@ -1471,10 +1457,6 @@ impl DiagCtxtInner {
self.panic_if_treat_err_as_bug();
}

fn bump_warn_count(&mut self) {
self.warn_count += 1;
}

fn panic_if_treat_err_as_bug(&self) {
if self.treat_err_as_bug() {
match (
Expand Down Expand Up @@ -1562,14 +1544,17 @@ pub enum Level {
/// Its `EmissionGuarantee` is `ErrorGuaranteed`.
Error,

/// A warning about the code being compiled. Does not prevent compilation from finishing.
/// A `force-warn` lint warning about the code being compiled. Does not prevent compilation
/// from finishing.
///
/// 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`.
/// The [`LintExpectationId`] is used for expected lint diagnostics. In all other cases this
/// should be `None`.
ForceWarning(Option<LintExpectationId>),

/// A warning about the code being compiled. Does not prevent compilation from finishing.
///
/// Its `EmissionGuarantee` is `()`.
Warning(Option<LintExpectationId>),
Warning,

/// A message giving additional context. Rare, because notes are more commonly attached to other
/// diagnostics such as errors.
Expand Down Expand Up @@ -1622,7 +1607,7 @@ impl Level {
Bug | DelayedBug | Fatal | Error => {
spec.set_fg(Some(Color::Red)).set_intense(true);
}
Warning(_) => {
ForceWarning(_) | Warning => {
spec.set_fg(Some(Color::Yellow)).set_intense(cfg!(windows));
}
Note | OnceNote => {
Expand All @@ -1641,7 +1626,7 @@ impl Level {
match self {
Bug | DelayedBug => "error: internal compiler error",
Fatal | Error => "error",
Warning(_) => "warning",
ForceWarning(_) | Warning => "warning",
Note | OnceNote => "note",
Help | OnceHelp => "help",
FailureNote => "failure-note",
Expand All @@ -1655,7 +1640,7 @@ impl Level {

pub fn get_expectation_id(&self) -> Option<LintExpectationId> {
match self {
Expect(id) | Warning(Some(id)) => Some(*id),
Expect(id) | ForceWarning(Some(id)) => Some(*id),
_ => None,
}
}
Expand Down
Loading
Loading