Skip to content

Commit

Permalink
Auto merge of #64272 - Mark-Simulacrum:parallel-handler, r=estebank
Browse files Browse the repository at this point in the history
Refactor librustc_errors::Handler API

This should be reviewed by-commit.

The last commit moves all fields into an inner struct behind a single lock; this is done to prevent possible deadlocks in a multi-threaded compiler, as well as inconsistent state observation.
  • Loading branch information
bors committed Sep 23, 2019
2 parents b6716a1 + 4cc5aaa commit 66bf391
Show file tree
Hide file tree
Showing 28 changed files with 322 additions and 262 deletions.
4 changes: 2 additions & 2 deletions src/librustc/dep_graph/graph.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use errors::{Diagnostic, DiagnosticBuilder};
use errors::Diagnostic;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
Expand Down Expand Up @@ -819,7 +819,7 @@ impl DepGraph {
let handle = tcx.sess.diagnostic();

for diagnostic in diagnostics {
DiagnosticBuilder::new_diagnostic(handle, diagnostic).emit();
handle.emit_diagnostic(&diagnostic);
}

// Mark the node as green now that diagnostics are emitted
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
Some((expected, found)) => Some((expected, found)),
None => {
// Derived error. Cancel the emitter.
self.tcx.sess.diagnostic().cancel(diag);
diag.cancel();
return;
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1855,7 +1855,7 @@ pub fn rustc_optgroups() -> Vec<RustcOptGroup> {
struct NullEmitter;

impl errors::emitter::Emitter for NullEmitter {
fn emit_diagnostic(&mut self, _: &errors::DiagnosticBuilder<'_>) {}
fn emit_diagnostic(&mut self, _: &errors::Diagnostic) {}
}

// Converts strings provided as `--cfg [cfgspec]` into a `crate_cfg`.
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/session/config/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn test_can_print_warnings() {
let registry = errors::registry::Registry::new(&[]);
let (sessopts, _) = build_session_options_and_crate_config(&matches);
let sess = build_session(sessopts, None, registry);
assert!(!sess.diagnostic().flags.can_emit_warnings);
assert!(!sess.diagnostic().can_emit_warnings());
});

syntax::with_default_globals(|| {
Expand All @@ -97,15 +97,15 @@ fn test_can_print_warnings() {
let registry = errors::registry::Registry::new(&[]);
let (sessopts, _) = build_session_options_and_crate_config(&matches);
let sess = build_session(sessopts, None, registry);
assert!(sess.diagnostic().flags.can_emit_warnings);
assert!(sess.diagnostic().can_emit_warnings());
});

syntax::with_default_globals(|| {
let matches = optgroups().parse(&["-Adead_code".to_string()]).unwrap();
let registry = errors::registry::Registry::new(&[]);
let (sessopts, _) = build_session_options_and_crate_config(&matches);
let sess = build_session(sessopts, None, registry);
assert!(sess.diagnostic().flags.can_emit_warnings);
assert!(sess.diagnostic().can_emit_warnings());
});
}

Expand Down
24 changes: 12 additions & 12 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,12 +365,6 @@ impl Session {
pub fn span_note_without_error<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
self.diagnostic().span_note_without_error(sp, msg)
}
pub fn span_unimpl<S: Into<MultiSpan>>(&self, sp: S, msg: &str) -> ! {
self.diagnostic().span_unimpl(sp, msg)
}
pub fn unimpl(&self, msg: &str) -> ! {
self.diagnostic().unimpl(msg)
}

pub fn buffer_lint<S: Into<MultiSpan>>(
&self,
Expand Down Expand Up @@ -1040,6 +1034,7 @@ fn default_emitter(
source_map: &Lrc<source_map::SourceMap>,
emitter_dest: Option<Box<dyn Write + Send>>,
) -> Box<dyn Emitter + sync::Send> {
let external_macro_backtrace = sopts.debugging_opts.external_macro_backtrace;
match (sopts.error_format, emitter_dest) {
(config::ErrorOutputType::HumanReadable(kind), dst) => {
let (short, color_config) = kind.unzip();
Expand All @@ -1048,6 +1043,7 @@ fn default_emitter(
let emitter = AnnotateSnippetEmitterWriter::new(
Some(source_map.clone()),
short,
external_macro_backtrace,
);
Box::new(emitter.ui_testing(sopts.debugging_opts.ui_testing))
} else {
Expand All @@ -1058,6 +1054,7 @@ fn default_emitter(
short,
sopts.debugging_opts.teach,
sopts.debugging_opts.terminal_width,
external_macro_backtrace,
),
Some(dst) => EmitterWriter::new(
dst,
Expand All @@ -1066,6 +1063,7 @@ fn default_emitter(
false, // no teach messages when writing to a buffer
false, // no colors when writing to a buffer
None, // no terminal width
external_macro_backtrace,
),
};
Box::new(emitter.ui_testing(sopts.debugging_opts.ui_testing))
Expand All @@ -1077,6 +1075,7 @@ fn default_emitter(
source_map.clone(),
pretty,
json_rendered,
external_macro_backtrace,
).ui_testing(sopts.debugging_opts.ui_testing),
),
(config::ErrorOutputType::Json { pretty, json_rendered }, Some(dst)) => Box::new(
Expand All @@ -1086,6 +1085,7 @@ fn default_emitter(
source_map.clone(),
pretty,
json_rendered,
external_macro_backtrace,
).ui_testing(sopts.debugging_opts.ui_testing),
),
}
Expand Down Expand Up @@ -1382,27 +1382,27 @@ pub fn early_error(output: config::ErrorOutputType, msg: &str) -> ! {
let emitter: Box<dyn Emitter + sync::Send> = match output {
config::ErrorOutputType::HumanReadable(kind) => {
let (short, color_config) = kind.unzip();
Box::new(EmitterWriter::stderr(color_config, None, short, false, None))
Box::new(EmitterWriter::stderr(color_config, None, short, false, None, false))
}
config::ErrorOutputType::Json { pretty, json_rendered } =>
Box::new(JsonEmitter::basic(pretty, json_rendered)),
Box::new(JsonEmitter::basic(pretty, json_rendered, false)),
};
let handler = errors::Handler::with_emitter(true, None, emitter);
handler.emit(&MultiSpan::new(), msg, errors::Level::Fatal);
handler.struct_fatal(msg).emit();
errors::FatalError.raise();
}

pub fn early_warn(output: config::ErrorOutputType, msg: &str) {
let emitter: Box<dyn Emitter + sync::Send> = match output {
config::ErrorOutputType::HumanReadable(kind) => {
let (short, color_config) = kind.unzip();
Box::new(EmitterWriter::stderr(color_config, None, short, false, None))
Box::new(EmitterWriter::stderr(color_config, None, short, false, None, false))
}
config::ErrorOutputType::Json { pretty, json_rendered } =>
Box::new(JsonEmitter::basic(pretty, json_rendered)),
Box::new(JsonEmitter::basic(pretty, json_rendered, false)),
};
let handler = errors::Handler::with_emitter(true, None, emitter);
handler.emit(&MultiSpan::new(), msg, errors::Level::Warning);
handler.struct_warn(msg).emit();
}

pub type CompileResult = Result<(), ErrorReported>;
7 changes: 3 additions & 4 deletions src/librustc/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,14 +330,13 @@ impl<'tcx> TyCtxt<'tcx> {
let mut i = 0;

while let Some(query) = current_query {
let mut db = DiagnosticBuilder::new(icx.tcx.sess.diagnostic(),
Level::FailureNote,
let mut diag = Diagnostic::new(Level::FailureNote,
&format!("#{} [{}] {}",
i,
query.info.query.name(),
query.info.query.describe(icx.tcx)));
db.set_span(icx.tcx.sess.source_map().def_span(query.info.span));
icx.tcx.sess.diagnostic().force_print_db(db);
diag.span = icx.tcx.sess.source_map().def_span(query.info.span).into();
icx.tcx.sess.diagnostic().force_print_diagnostic(diag);

current_query = query.parent.clone();
i += 1;
Expand Down
22 changes: 7 additions & 15 deletions src/librustc_codegen_ssa/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,11 @@ use rustc::util::common::{time_depth, set_time_depth, print_time_passes_entry};
use rustc::util::profiling::SelfProfiler;
use rustc_fs_util::link_or_copy;
use rustc_data_structures::svh::Svh;
use rustc_errors::{Handler, Level, DiagnosticBuilder, FatalError, DiagnosticId};
use rustc_errors::{Handler, Level, FatalError, DiagnosticId};
use rustc_errors::emitter::{Emitter};
use rustc_target::spec::MergeFunctions;
use syntax::attr;
use syntax::ext::hygiene::ExpnId;
use syntax_pos::MultiSpan;
use syntax_pos::symbol::{Symbol, sym};
use jobserver::{Client, Acquired};

Expand Down Expand Up @@ -1725,7 +1724,7 @@ impl SharedEmitter {
}

impl Emitter for SharedEmitter {
fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>) {
fn emit_diagnostic(&mut self, db: &rustc_errors::Diagnostic) {
drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic {
msg: db.message(),
code: db.code.clone(),
Expand Down Expand Up @@ -1760,19 +1759,12 @@ impl SharedEmitterMain {
match message {
Ok(SharedEmitterMessage::Diagnostic(diag)) => {
let handler = sess.diagnostic();
match diag.code {
Some(ref code) => {
handler.emit_with_code(&MultiSpan::new(),
&diag.msg,
code.clone(),
diag.lvl);
}
None => {
handler.emit(&MultiSpan::new(),
&diag.msg,
diag.lvl);
}
let mut d = rustc_errors::Diagnostic::new(diag.lvl, &diag.msg);
if let Some(code) = diag.code {
d.code(code);
}
handler.emit_diagnostic(&d);
handler.abort_if_errors_and_should_abort();
}
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg)) => {
sess.span_err(ExpnId::from_u32(cookie).expn_data().call_site, &msg)
Expand Down
13 changes: 6 additions & 7 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use syntax::source_map::FileLoader;
use syntax::feature_gate::{GatedCfg, UnstableFeatures};
use syntax::parse::{self, PResult};
use syntax::symbol::sym;
use syntax_pos::{DUMMY_SP, MultiSpan, FileName};
use syntax_pos::{DUMMY_SP, FileName};

pub mod pretty;
mod args;
Expand Down Expand Up @@ -1196,15 +1196,16 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
false,
false,
None,
false,
));
let handler = errors::Handler::with_emitter(true, None, emitter);

// a .span_bug or .bug call has already printed what
// it wants to print.
if !info.payload().is::<errors::ExplicitBug>() {
handler.emit(&MultiSpan::new(),
"unexpected panic",
errors::Level::Bug);
let d = errors::Diagnostic::new(errors::Level::Bug, "unexpected panic");
handler.emit_diagnostic(&d);
handler.abort_if_errors_and_should_abort();
}

let mut xs: Vec<Cow<'static, str>> = vec![
Expand All @@ -1224,9 +1225,7 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
}

for note in &xs {
handler.emit(&MultiSpan::new(),
note,
errors::Level::Note);
handler.note_without_error(&note);
}

// If backtraces are enabled, also print the query stack
Expand Down
12 changes: 8 additions & 4 deletions src/librustc_errors/annotate_snippet_emitter_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use syntax_pos::{SourceFile, MultiSpan, Loc};
use crate::{
Level, CodeSuggestion, DiagnosticBuilder, Emitter,
Level, CodeSuggestion, Diagnostic, Emitter,
SourceMapperDyn, SubDiagnostic, DiagnosticId
};
use crate::emitter::FileWithAnnotatedLines;
Expand All @@ -25,19 +25,21 @@ pub struct AnnotateSnippetEmitterWriter {
short_message: bool,
/// If true, will normalize line numbers with `LL` to prevent noise in UI test diffs.
ui_testing: bool,

external_macro_backtrace: bool,
}

impl Emitter for AnnotateSnippetEmitterWriter {
/// The entry point for the diagnostics generation
fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>) {
fn emit_diagnostic(&mut self, db: &Diagnostic) {
let mut children = db.children.clone();
let (mut primary_span, suggestions) = self.primary_span_formatted(&db);

self.fix_multispans_in_std_macros(&self.source_map,
&mut primary_span,
&mut children,
&db.level,
db.handler().flags.external_macro_backtrace);
self.external_macro_backtrace);

self.emit_messages_default(&db.level,
db.message(),
Expand Down Expand Up @@ -163,12 +165,14 @@ impl<'a> DiagnosticConverter<'a> {
impl AnnotateSnippetEmitterWriter {
pub fn new(
source_map: Option<Lrc<SourceMapperDyn>>,
short_message: bool
short_message: bool,
external_macro_backtrace: bool,
) -> Self {
Self {
source_map,
short_message,
ui_testing: false,
external_macro_backtrace,
}
}

Expand Down
15 changes: 4 additions & 11 deletions src/librustc_errors/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,17 +99,9 @@ impl<'a> DerefMut for DiagnosticBuilder<'a> {
}

impl<'a> DiagnosticBuilder<'a> {
pub fn handler(&self) -> &'a Handler{
self.0.handler
}

/// Emit the diagnostic.
pub fn emit(&mut self) {
if self.cancelled() {
return;
}

self.0.handler.emit_db(&self);
self.0.handler.emit_diagnostic(&self);
self.cancel();
}

Expand Down Expand Up @@ -354,7 +346,7 @@ impl<'a> DiagnosticBuilder<'a> {

/// Convenience function for internal use, clients should use one of the
/// struct_* methods on Handler.
pub fn new(handler: &'a Handler, level: Level, message: &str) -> DiagnosticBuilder<'a> {
crate fn new(handler: &'a Handler, level: Level, message: &str) -> DiagnosticBuilder<'a> {
DiagnosticBuilder::new_with_code(handler, level, None, message)
}

Expand All @@ -371,7 +363,8 @@ impl<'a> DiagnosticBuilder<'a> {

/// Creates a new `DiagnosticBuilder` with an already constructed
/// diagnostic.
pub fn new_diagnostic(handler: &'a Handler, diagnostic: Diagnostic) -> DiagnosticBuilder<'a> {
crate fn new_diagnostic(handler: &'a Handler, diagnostic: Diagnostic)
-> DiagnosticBuilder<'a> {
DiagnosticBuilder(Box::new(DiagnosticBuilderInner {
handler,
diagnostic,
Expand Down
Loading

0 comments on commit 66bf391

Please sign in to comment.