From 68f5101cd5b2cb51ac2873f9d28c1f5f3ce842e3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 12 Apr 2019 16:27:36 +1000 Subject: [PATCH 1/3] Remove unused `DiagnosticOutput::Emitter` variant. --- src/librustc/session/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 54eb1d815d31a..ad8825bc5de5a 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -1078,8 +1078,7 @@ fn default_emitter( pub enum DiagnosticOutput { Default, - Raw(Box), - Emitter(Box) + Raw(Box) } pub fn build_session_with_source_map( @@ -1115,7 +1114,6 @@ pub fn build_session_with_source_map( DiagnosticOutput::Raw(write) => { default_emitter(&sopts, registry, &source_map, Some(write)) } - DiagnosticOutput::Emitter(emitter) => emitter, }; let diagnostic_handler = errors::Handler::with_emitter_and_flags( From 91d5b764ea1e717641b146d5c3169058a18f3919 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 24 Apr 2019 15:02:56 +1000 Subject: [PATCH 2/3] Remove some unused return values. --- src/librustc_codegen_ssa/back/link.rs | 40 ++++++++++----------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index fe703ff4d25dd..b5e41dd22c9f0 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -46,11 +46,10 @@ pub fn remove(sess: &Session, path: &Path) { /// Performs the linkage portion of the compilation phase. This will generate all /// of the requested outputs for this compilation session. pub fn link_binary<'a, B: ArchiveBuilder<'a>>(sess: &'a Session, - codegen_results: &CodegenResults, - outputs: &OutputFilenames, - crate_name: &str, - target_cpu: &str) -> Vec { - let mut out_filenames = Vec::new(); + codegen_results: &CodegenResults, + outputs: &OutputFilenames, + crate_name: &str, + target_cpu: &str) { for &crate_type in sess.crate_types.borrow().iter() { // Ignore executable crates if we have -Z no-codegen, as they will error. let output_metadata = sess.opts.output_types.contains_key(&OutputType::Metadata); @@ -64,13 +63,12 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(sess: &'a Session, bug!("invalid output type `{:?}` for target os `{}`", crate_type, sess.opts.target_triple); } - let out_files = link_binary_output::(sess, - codegen_results, - crate_type, - outputs, - crate_name, - target_cpu); - out_filenames.extend(out_files); + link_binary_output::(sess, + codegen_results, + crate_type, + outputs, + crate_name, + target_cpu); } // Remove the temporary object file and metadata if we aren't saving temps @@ -97,22 +95,18 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(sess: &'a Session, } } } - - out_filenames } fn link_binary_output<'a, B: ArchiveBuilder<'a>>(sess: &'a Session, - codegen_results: &CodegenResults, - crate_type: config::CrateType, - outputs: &OutputFilenames, - crate_name: &str, - target_cpu: &str) -> Vec { + codegen_results: &CodegenResults, + crate_type: config::CrateType, + outputs: &OutputFilenames, + crate_name: &str, + target_cpu: &str) { for obj in codegen_results.modules.iter().filter_map(|m| m.object.as_ref()) { check_file_is_writeable(obj, sess); } - let mut out_filenames = vec![]; - if outputs.outputs.contains_key(&OutputType::Metadata) { let out_filename = filename_for_metadata(sess, crate_name, outputs); // To avoid races with another rustc process scanning the output directory, @@ -128,7 +122,6 @@ fn link_binary_output<'a, B: ArchiveBuilder<'a>>(sess: &'a Session, if let Err(e) = fs::rename(metadata, &out_filename) { sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e)); } - out_filenames.push(out_filename); } let tmpdir = TempFileBuilder::new().prefix("rustc").tempdir().unwrap_or_else(|err| @@ -158,14 +151,11 @@ fn link_binary_output<'a, B: ArchiveBuilder<'a>>(sess: &'a Session, ); } } - out_filenames.push(out_filename); } if sess.opts.cg.save_temps { let _ = tmpdir.into_path(); } - - out_filenames } // The third parameter is for env vars, used on windows to set up the From 7bcb0cffb61d5e1e4fbe08a51d92883556daed04 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 15 Apr 2019 08:26:08 +1000 Subject: [PATCH 3/3] In JSON output, emit a directive after metadata is generated. To implement pipelining, Cargo needs to know when metadata generation is finished. This commit adds code to do that. Unfortunately, metadata file writing currently occurs very late during compilation, so pipelining won't produce a speed-up. Moving metadata file writing earlier will be a follow-up. The change involves splitting the existing `Emitter::emit` method in two: `Emitter::emit_diagnostic` and `Emitter::emit_directive`. The JSON directives look like this: ``` {"directive":"metadata file written: liba.rmeta"} ``` The functionality is behind the `-Z emit-directives` option, and also requires `--error-format=json`. --- src/librustc/session/config.rs | 2 ++ src/librustc_codegen_llvm/lib.rs | 2 +- src/librustc_codegen_ssa/back/link.rs | 10 ++++++++-- src/librustc_codegen_ssa/back/write.rs | 4 ++-- src/librustc_errors/emitter.rs | 8 ++++++-- src/librustc_errors/lib.rs | 22 +++++++++++++++------ src/libsyntax/json.rs | 22 +++++++++++++++++++-- src/test/ui/emit-directives.rs | 12 ++++++++++++ src/test/ui/emit-directives.stderr | 1 + src/tools/compiletest/src/json.rs | 27 ++++++++++++++++---------- 10 files changed, 85 insertions(+), 25 deletions(-) create mode 100644 src/test/ui/emit-directives.rs create mode 100644 src/test/ui/emit-directives.stderr diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index cb307800fcdc2..4fef6cd3dd290 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1468,6 +1468,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, the same values as the target option of the same name"), allow_features: Option> = (None, parse_opt_comma_list, [TRACKED], "only allow the listed language features to be enabled in code (space separated)"), + emit_directives: bool = (false, parse_bool, [UNTRACKED], + "emit build directives if producing JSON output"), } pub fn default_lib_output() -> CrateType { diff --git a/src/librustc_codegen_llvm/lib.rs b/src/librustc_codegen_llvm/lib.rs index cee0d5be6473b..08424e7c3229a 100644 --- a/src/librustc_codegen_llvm/lib.rs +++ b/src/librustc_codegen_llvm/lib.rs @@ -300,7 +300,7 @@ impl CodegenBackend for LlvmCodegenBackend { sess: &Session, dep_graph: &DepGraph, outputs: &OutputFilenames, - ) -> Result<(), ErrorReported>{ + ) -> Result<(), ErrorReported> { use rustc::util::common::time; let (codegen_results, work_products) = ongoing_codegen.downcast:: diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index b5e41dd22c9f0..4cae20b698a1c 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -119,8 +119,14 @@ fn link_binary_output<'a, B: ArchiveBuilder<'a>>(sess: &'a Session, .tempdir_in(out_filename.parent().unwrap()) .unwrap_or_else(|err| sess.fatal(&format!("couldn't create a temp dir: {}", err))); let metadata = emit_metadata(sess, codegen_results, &metadata_tmpdir); - if let Err(e) = fs::rename(metadata, &out_filename) { - sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e)); + match fs::rename(&metadata, &out_filename) { + Ok(_) => { + if sess.opts.debugging_opts.emit_directives { + sess.parse_sess.span_diagnostic.maybe_emit_json_directive( + format!("metadata file written: {}", out_filename.display())); + } + } + Err(e) => sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e)), } } diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index 88a5e5a1aec3b..576bcc8f38e65 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -1726,7 +1726,7 @@ impl SharedEmitter { } impl Emitter for SharedEmitter { - fn emit(&mut self, db: &DiagnosticBuilder<'_>) { + fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>) { drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { msg: db.message(), code: db.code.clone(), @@ -1865,7 +1865,7 @@ impl OngoingCodegen { self.wait_for_signal_to_codegen_item(); self.check_for_errors(tcx.sess); - // These are generally cheap and won't through off scheduling. + // These are generally cheap and won't throw off scheduling. let cost = 0; submit_codegened_module_to_llvm(&self.backend, tcx, module, cost); } diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index c3d594204f413..bfc9113c2d41e 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -50,7 +50,11 @@ const ANONYMIZED_LINE_NUM: &str = "LL"; /// Emitter trait for emitting errors. pub trait Emitter { /// Emit a structured diagnostic. - fn emit(&mut self, db: &DiagnosticBuilder<'_>); + fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>); + + /// Emit a JSON directive. The default is to do nothing; this should only + /// be emitted with --error-format=json. + fn maybe_emit_json_directive(&mut self, _directive: String) {} /// Checks if should show explanations about "rustc --explain" fn should_show_explain(&self) -> bool { @@ -59,7 +63,7 @@ pub trait Emitter { } impl Emitter for EmitterWriter { - fn emit(&mut self, db: &DiagnosticBuilder<'_>) { + fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>) { let mut primary_span = db.span.clone(); let mut children = db.children.clone(); let mut suggestions: &[_] = &[]; diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index 5d3861d9572ca..e173e1060cc10 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -294,9 +294,16 @@ impl error::Error for ExplicitBug { pub use diagnostic::{Diagnostic, SubDiagnostic, DiagnosticStyledString, DiagnosticId}; pub use diagnostic_builder::DiagnosticBuilder; -/// A handler deals with errors; certain errors -/// (fatal, bug, unimpl) may cause immediate exit, -/// others log errors for later reporting. +/// A handler deals with two kinds of compiler output. +/// - Errors: certain errors (fatal, bug, unimpl) may cause immediate exit, +/// others log errors for later reporting. +/// - Directives: with --error-format=json, the compiler produces directives +/// that indicate when certain actions have completed, which are useful for +/// Cargo. They may change at any time and should not be considered a public +/// API. +/// +/// This crate's name (rustc_errors) doesn't encompass the directives, because +/// directives were added much later. pub struct Handler { pub flags: HandlerFlags, @@ -736,7 +743,7 @@ impl Handler { } pub fn force_print_db(&self, mut db: DiagnosticBuilder<'_>) { - self.emitter.borrow_mut().emit(&db); + self.emitter.borrow_mut().emit_diagnostic(&db); db.cancel(); } @@ -761,14 +768,17 @@ impl Handler { // Only emit the diagnostic if we haven't already emitted an equivalent // one: if self.emitted_diagnostics.borrow_mut().insert(diagnostic_hash) { - self.emitter.borrow_mut().emit(db); + self.emitter.borrow_mut().emit_diagnostic(db); if db.is_error() { self.bump_err_count(); } } } -} + pub fn maybe_emit_json_directive(&self, directive: String) { + self.emitter.borrow_mut().maybe_emit_json_directive(directive); + } +} #[derive(Copy, PartialEq, Clone, Hash, Debug, RustcEncodable, RustcDecodable)] pub enum Level { diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs index c19b408442ad1..65f8d0e77d7be 100644 --- a/src/libsyntax/json.rs +++ b/src/libsyntax/json.rs @@ -79,7 +79,7 @@ impl JsonEmitter { } impl Emitter for JsonEmitter { - fn emit(&mut self, db: &DiagnosticBuilder<'_>) { + fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>) { let data = Diagnostic::from_diagnostic_builder(db, self); let result = if self.pretty { writeln!(&mut self.dst, "{}", as_pretty_json(&data)) @@ -90,6 +90,18 @@ impl Emitter for JsonEmitter { panic!("failed to print diagnostics: {:?}", e); } } + + fn maybe_emit_json_directive(&mut self, directive: String) { + let data = Directive { directive }; + let result = if self.pretty { + writeln!(&mut self.dst, "{}", as_pretty_json(&data)) + } else { + writeln!(&mut self.dst, "{}", as_json(&data)) + }; + if let Err(e) = result { + panic!("failed to print message: {:?}", e); + } + } } // The following data types are provided just for serialisation. @@ -168,6 +180,12 @@ struct DiagnosticCode { explanation: Option<&'static str>, } +#[derive(RustcEncodable)] +struct Directive { + /// The directive itself. + directive: String, +} + impl Diagnostic { fn from_diagnostic_builder(db: &DiagnosticBuilder<'_>, je: &JsonEmitter) @@ -200,7 +218,7 @@ impl Diagnostic { let buf = BufWriter::default(); let output = buf.clone(); je.json_rendered.new_emitter(Box::new(buf), Some(je.sm.clone()), false) - .ui_testing(je.ui_testing).emit(db); + .ui_testing(je.ui_testing).emit_diagnostic(db); let output = Arc::try_unwrap(output.0).unwrap().into_inner().unwrap(); let output = String::from_utf8(output).unwrap(); diff --git a/src/test/ui/emit-directives.rs b/src/test/ui/emit-directives.rs new file mode 100644 index 0000000000000..088280e358ae7 --- /dev/null +++ b/src/test/ui/emit-directives.rs @@ -0,0 +1,12 @@ +// ignore-tidy-linelength +// compile-flags:--emit=metadata --error-format=json -Z emit-directives +// compile-pass +// +// Normalization is required to eliminated minor path and filename differences +// across platforms. +// normalize-stderr-test: "metadata file written: .*/emit-directives" -> "metadata file written: .../emit-directives" +// normalize-stderr-test: "emit-directives(\.\w*)?/a(\.\w*)?" -> "emit-directives/a" + +// A very basic test for the emission of build directives in JSON output. + +fn main() {} diff --git a/src/test/ui/emit-directives.stderr b/src/test/ui/emit-directives.stderr new file mode 100644 index 0000000000000..b8a4b96f4bf25 --- /dev/null +++ b/src/test/ui/emit-directives.stderr @@ -0,0 +1 @@ +{"directive":"metadata file written: .../emit-directives/a"} diff --git a/src/tools/compiletest/src/json.rs b/src/tools/compiletest/src/json.rs index a7615f5f423a3..26a3c4dee40aa 100644 --- a/src/tools/compiletest/src/json.rs +++ b/src/tools/compiletest/src/json.rs @@ -17,6 +17,12 @@ struct Diagnostic { rendered: Option, } +#[derive(Deserialize)] +struct Directive { + #[allow(dead_code)] + directive: String, +} + #[derive(Deserialize, Clone)] struct DiagnosticSpan { file_name: String, @@ -67,16 +73,17 @@ pub fn extract_rendered(output: &str) -> String { .lines() .filter_map(|line| { if line.starts_with('{') { - match serde_json::from_str::(line) { - Ok(diagnostic) => diagnostic.rendered, - Err(error) => { - print!( - "failed to decode compiler output as json: \ - `{}`\nline: {}\noutput: {}", - error, line, output - ); - panic!() - } + if let Ok(diagnostic) = serde_json::from_str::(line) { + diagnostic.rendered + } else if let Ok(_directive) = serde_json::from_str::(line) { + // Swallow the directive. + None + } else { + print!( + "failed to decode compiler output as json: line: {}\noutput: {}", + line, output + ); + panic!() } } else { // preserve non-JSON lines, such as ICEs