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

Remove Session::one_time_diagnostic #95149

Merged
merged 4 commits into from
Mar 26, 2022
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
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2361,8 +2361,8 @@ mod error {
if !self.errors.buffered.is_empty() {
self.errors.buffered.sort_by_key(|diag| diag.sort_span);

for diag in self.errors.buffered.drain(..) {
self.infcx.tcx.sess.diagnostic().emit_diagnostic(&diag);
for mut diag in self.errors.buffered.drain(..) {
self.infcx.tcx.sess.diagnostic().emit_diagnostic(&mut diag);
}
}

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 @@ -1748,7 +1748,7 @@ impl SharedEmitterMain {
if let Some(code) = diag.code {
d.code(code);
}
handler.emit_diagnostic(&d);
handler.emit_diagnostic(&mut d);
}
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
let msg = msg.strip_prefix("error: ").unwrap_or(&msg);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
// "secondary" errors if they occurred.
let secondary_errors = mem::take(&mut self.secondary_errors);
if self.error_emitted.is_none() {
for error in secondary_errors {
self.tcx.sess.diagnostic().emit_diagnostic(&error);
for mut error in secondary_errors {
self.tcx.sess.diagnostic().emit_diagnostic(&mut error);
}
} else {
assert!(self.tcx.sess.has_errors().is_some());
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1181,8 +1181,8 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
// a .span_bug or .bug call has already printed what
// it wants to print.
if !info.payload().is::<rustc_errors::ExplicitBug>() {
let d = rustc_errors::Diagnostic::new(rustc_errors::Level::Bug, "unexpected panic");
handler.emit_diagnostic(&d);
let mut d = rustc_errors::Diagnostic::new(rustc_errors::Level::Bug, "unexpected panic");
handler.emit_diagnostic(&mut d);
}

let mut xs: Vec<Cow<'static, str>> = vec![
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn annotation_type_for_level(level: Level) -> AnnotationType {
AnnotationType::Error
}
Level::Warning => AnnotationType::Warning,
Level::Note => AnnotationType::Note,
Level::Note | Level::OnceNote => AnnotationType::Note,
Level::Help => AnnotationType::Help,
// FIXME(#59346): Not sure how to map this level
Level::FailureNote => AnnotationType::Error,
Expand Down
21 changes: 20 additions & 1 deletion compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,12 @@ impl Diagnostic {
| Level::Error { .. }
| Level::FailureNote => true,

Level::Warning | Level::Note | Level::Help | Level::Allow | Level::Expect(_) => false,
Level::Warning
| Level::Note
| Level::OnceNote
| Level::Help
| Level::Allow
| Level::Expect(_) => false,
}
}

Expand Down Expand Up @@ -333,13 +338,27 @@ impl Diagnostic {
self
}

/// Prints the span with a note above it.
/// This is like [`Diagnostic::note()`], but it gets its own span.
pub fn note_once(&mut self, msg: &str) -> &mut Self {
self.sub(Level::OnceNote, msg, MultiSpan::new(), None);
self
}

/// Prints the span with a note above it.
/// This is like [`Diagnostic::note()`], but it gets its own span.
pub fn span_note<S: Into<MultiSpan>>(&mut self, sp: S, msg: &str) -> &mut Self {
self.sub(Level::Note, msg, sp.into(), None);
self
}

/// Prints the span with a note above it.
/// This is like [`Diagnostic::note()`], but it gets its own span.
pub fn span_note_once<S: Into<MultiSpan>>(&mut self, sp: S, msg: &str) -> &mut Self {
self.sub(Level::OnceNote, msg, sp.into(), None);
self
}

/// Add a warning attached to this diagnostic.
pub fn warn(&mut self, msg: &str) -> &mut Self {
self.sub(Level::Warning, msg, MultiSpan::new(), None);
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl EmissionGuarantee for ErrorGuaranteed {
DiagnosticBuilderState::Emittable(handler) => {
db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;

let guar = handler.emit_diagnostic(&db.inner.diagnostic);
let guar = handler.emit_diagnostic(&mut db.inner.diagnostic);

// Only allow a guarantee if the `level` wasn't switched to a
// non-error - the field isn't `pub`, but the whole `Diagnostic`
Expand Down Expand Up @@ -190,7 +190,7 @@ impl EmissionGuarantee for () {
DiagnosticBuilderState::Emittable(handler) => {
db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation;

handler.emit_diagnostic(&db.inner.diagnostic);
handler.emit_diagnostic(&mut db.inner.diagnostic);
}
// `.emit()` was previously called, disallowed from repeating it.
DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
Expand Down Expand Up @@ -396,11 +396,17 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
) -> &mut Self);

forward!(pub fn note(&mut self, msg: &str) -> &mut Self);
forward!(pub fn note_once(&mut self, msg: &str) -> &mut Self);
forward!(pub fn span_note(
&mut self,
sp: impl Into<MultiSpan>,
msg: &str,
) -> &mut Self);
forward!(pub fn span_note_once(
&mut self,
sp: impl Into<MultiSpan>,
msg: &str,
) -> &mut Self);
forward!(pub fn warn(&mut self, msg: &str) -> &mut Self);
forward!(pub fn span_warn(&mut self, sp: impl Into<MultiSpan>, msg: &str) -> &mut Self);
forward!(pub fn help(&mut self, msg: &str) -> &mut Self);
Expand Down Expand Up @@ -500,11 +506,11 @@ impl Drop for DiagnosticBuilderInner<'_> {
// No `.emit()` or `.cancel()` calls.
DiagnosticBuilderState::Emittable(handler) => {
if !panicking() {
handler.emit_diagnostic(&Diagnostic::new(
handler.emit_diagnostic(&mut Diagnostic::new(
Level::Bug,
"the following error was constructed but not emitted",
));
handler.emit_diagnostic(&self.diagnostic);
handler.emit_diagnostic(&mut self.diagnostic);
panic!();
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ impl Emitter for SilentEmitter {
if let Some(ref note) = self.fatal_note {
d.note(note);
}
self.fatal_handler.emit_diagnostic(&d);
self.fatal_handler.emit_diagnostic(&mut d);
}
}
}
Expand Down
55 changes: 37 additions & 18 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![feature(crate_visibility_modifier)]
#![feature(drain_filter)]
#![feature(backtrace)]
#![feature(if_let_guard)]
#![feature(let_else)]
Expand Down Expand Up @@ -919,7 +920,7 @@ impl Handler {
self.inner.borrow_mut().force_print_diagnostic(db)
}

pub fn emit_diagnostic(&self, diagnostic: &Diagnostic) -> Option<ErrorGuaranteed> {
pub fn emit_diagnostic(&self, diagnostic: &mut Diagnostic) -> Option<ErrorGuaranteed> {
self.inner.borrow_mut().emit_diagnostic(diagnostic)
}

Expand Down Expand Up @@ -993,25 +994,25 @@ impl HandlerInner {
self.taught_diagnostics.insert(code.clone())
}

fn force_print_diagnostic(&mut self, db: Diagnostic) {
self.emitter.emit_diagnostic(&db);
fn force_print_diagnostic(&mut self, mut db: Diagnostic) {
self.emitter.emit_diagnostic(&mut db);
}

/// Emit all stashed diagnostics.
fn emit_stashed_diagnostics(&mut self) -> Option<ErrorGuaranteed> {
let diags = self.stashed_diagnostics.drain(..).map(|x| x.1).collect::<Vec<_>>();
let mut reported = None;
diags.iter().for_each(|diag| {
for mut diag in diags {
if diag.is_error() {
reported = Some(ErrorGuaranteed(()));
}
self.emit_diagnostic(diag);
});
self.emit_diagnostic(&mut diag);
}
reported
}

// FIXME(eddyb) this should ideally take `diagnostic` by value.
fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) -> Option<ErrorGuaranteed> {
fn emit_diagnostic(&mut self, diagnostic: &mut Diagnostic) -> Option<ErrorGuaranteed> {
Comment on lines 1014 to +1015
Copy link
Contributor

Choose a reason for hiding this comment

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

Would fixing the FIXME be possible as part of this PR, or are there big consequences that I'm failing to think of?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This requires to make the DiagnosticBuilder::emit method take self by move, which breaks struct_err(..).label(..).emit() chains.

Copy link
Member

Choose a reason for hiding this comment

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

Not exactly, I decided we can't make .emit() take &mut self for that reason, but once .emit() is called the DiagnosticBuilder state changes to ignore the Diagnostic on further .emit(), so we could be replacing the Diagnostic with a dummy one (or even having Option around it but that could slow down the Deref/DerefMut impl a bunch).

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 Down Expand Up @@ -1070,7 +1071,23 @@ impl HandlerInner {
// Only emit the diagnostic if we've been asked to deduplicate and
// haven't already emitted an equivalent diagnostic.
if !(self.flags.deduplicate_diagnostics && already_emitted(self)) {
self.emitter.emit_diagnostic(diagnostic);
debug!(?diagnostic);
debug!(?self.emitted_diagnostics);
let already_emitted_sub = |sub: &mut SubDiagnostic| {
debug!(?sub);
if sub.level != Level::OnceNote {
return false;
}
let mut hasher = StableHasher::new();
sub.hash(&mut hasher);
let diagnostic_hash = hasher.finish();
debug!(?diagnostic_hash);
!self.emitted_diagnostics.insert(diagnostic_hash)
};

diagnostic.children.drain_filter(already_emitted_sub).for_each(|_| {});

self.emitter.emit_diagnostic(&diagnostic);
if diagnostic.is_error() {
self.deduplicated_err_count += 1;
} else if diagnostic.level == Warning {
Expand Down Expand Up @@ -1221,22 +1238,22 @@ impl HandlerInner {
let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg);
diagnostic.set_span(sp.into());
diagnostic.note(&format!("delayed at {}", std::panic::Location::caller()));
self.emit_diagnostic(&diagnostic).unwrap()
self.emit_diagnostic(&mut diagnostic).unwrap()
}

// FIXME(eddyb) note the comment inside `impl Drop for HandlerInner`, that's
// where the explanation of what "good path" is (also, it should be renamed).
fn delay_good_path_bug(&mut self, msg: &str) {
let diagnostic = Diagnostic::new(Level::DelayedBug, msg);
let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg);
if self.flags.report_delayed_bugs {
self.emit_diagnostic(&diagnostic);
self.emit_diagnostic(&mut diagnostic);
}
let backtrace = std::backtrace::Backtrace::force_capture();
self.delayed_good_path_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace));
}

fn failure(&mut self, msg: &str) {
self.emit_diagnostic(&Diagnostic::new(FailureNote, msg));
self.emit_diagnostic(&mut Diagnostic::new(FailureNote, msg));
}

fn fatal(&mut self, msg: &str) -> FatalError {
Expand All @@ -1253,11 +1270,11 @@ impl HandlerInner {
if self.treat_err_as_bug() {
self.bug(msg);
}
self.emit_diagnostic(&Diagnostic::new(level, msg)).unwrap()
self.emit_diagnostic(&mut Diagnostic::new(level, msg)).unwrap()
}

fn bug(&mut self, msg: &str) -> ! {
self.emit_diagnostic(&Diagnostic::new(Bug, msg));
self.emit_diagnostic(&mut Diagnostic::new(Bug, msg));
panic::panic_any(ExplicitBug);
}

Expand All @@ -1267,7 +1284,7 @@ impl HandlerInner {
if no_bugs {
// Put the overall explanation before the `DelayedBug`s, to
// frame them better (e.g. separate warnings from them).
self.emit_diagnostic(&Diagnostic::new(Bug, explanation));
self.emit_diagnostic(&mut Diagnostic::new(Bug, explanation));
no_bugs = false;
}

Expand All @@ -1283,7 +1300,7 @@ impl HandlerInner {
}
bug.level = Level::Bug;

self.emit_diagnostic(&bug);
self.emit_diagnostic(&mut bug);
}

// Panic with `ExplicitBug` to avoid "unexpected panic" messages.
Expand Down Expand Up @@ -1350,6 +1367,8 @@ pub enum Level {
},
Warning,
Note,
/// A note that is only emitted once.
OnceNote,
Help,
FailureNote,
Allow,
Expand All @@ -1372,7 +1391,7 @@ impl Level {
Warning => {
spec.set_fg(Some(Color::Yellow)).set_intense(cfg!(windows));
}
Note => {
Note | OnceNote => {
spec.set_fg(Some(Color::Green)).set_intense(true);
}
Help => {
Expand All @@ -1389,7 +1408,7 @@ impl Level {
Bug | DelayedBug => "error: internal compiler error",
Fatal | Error { .. } => "error",
Warning => "warning",
Note => "note",
Note | OnceNote => "note",
Help => "help",
FailureNote => "failure-note",
Allow => panic!("Shouldn't call on allowed error"),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_expand/src/proc_macro_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,8 +771,8 @@ impl server::Diagnostic for Rustc<'_, '_> {
) {
diag.sub(level.to_internal(), msg, MultiSpan::from_spans(spans), None);
}
fn emit(&mut self, diag: Self::Diagnostic) {
self.sess().span_diagnostic.emit_diagnostic(&diag);
fn emit(&mut self, mut diag: Self::Diagnostic) {
self.sess().span_diagnostic.emit_diagnostic(&mut diag);
}
}

Expand Down
Loading