Skip to content

Commit

Permalink
Unrolled build for rust-lang#119763
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#119763 - nnethercote:cleanup-Diagnostic, r=oli-obk

Cleanup things in and around `Diagnostic`

These changes all arose when I was looking closely at how to simplify `DiagCtxtInner::emit_diagnostic`.

r? `@compiler-errors`
  • Loading branch information
rust-timer authored Jan 11, 2024
2 parents 0a89233 + dd61eba commit ca828a1
Show file tree
Hide file tree
Showing 13 changed files with 72 additions and 95 deletions.
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

0 comments on commit ca828a1

Please sign in to comment.