Skip to content

Commit

Permalink
Auto merge of #60464 - eddyb:not-overly-specific-pipelining, r=alexcr…
Browse files Browse the repository at this point in the history
…ichton

rustc: rename -Z emit-directives to -Z emit-artifact-notifications and simplify the output.

This is my take on #60006 / #60419 (see #60006 (comment)).
I'm not too attached the "notifications" part, it's pretty much bikeshed material.
**EDIT**: for "artifact", @matklad pointed out Cargo already uses it (in #60464 (comment))

The first two commits are fixes that could be landed independently, especially the `compiletest` one, which removes the need for any of the normalization added in #60006 to land the test.

The last commit enables the emission for all outputs, which was my main suggestion for #60006, mostly to show that it's minimal and not really a "scope creep" (as suggested in #60006 (comment)).

cc @alexcrichton @nnethercote
  • Loading branch information
bors committed May 7, 2019
2 parents eeedd3a + c89a131 commit b8fa4cb
Show file tree
Hide file tree
Showing 16 changed files with 87 additions and 74 deletions.
4 changes: 2 additions & 2 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1462,8 +1462,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
the same values as the target option of the same name"),
allow_features: Option<Vec<String>> = (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"),
emit_artifact_notifications: bool = (false, parse_bool, [UNTRACKED],
"emit notifications after each artifact has been output (only in the JSON format)"),
}

pub fn default_lib_output() -> CrateType {
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_codegen_ssa/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(sess: &'a Session,
);
}
}
if sess.opts.debugging_opts.emit_artifact_notifications {
sess.parse_sess.span_diagnostic.emit_artifact_notification(&out_filename);
}
}

if sess.opts.cg.save_temps {
Expand Down
8 changes: 5 additions & 3 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::borrow::Cow;
use std::io::prelude::*;
use std::io;
use std::cmp::{min, Reverse};
use std::path::Path;
use termcolor::{StandardStream, ColorChoice, ColorSpec, BufferWriter, Ansi};
use termcolor::{WriteColor, Color, Buffer};

Expand Down Expand Up @@ -52,9 +53,10 @@ pub trait Emitter {
/// Emit a structured diagnostic.
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) {}
/// Emit a notification that an artifact has been output.
/// This is currently only supported for the JSON format,
/// other formats can, and will, simply ignore it.
fn emit_artifact_notification(&mut self, _path: &Path) {}

/// Checks if should show explanations about "rustc --explain"
fn should_show_explain(&self) -> bool {
Expand Down
18 changes: 6 additions & 12 deletions src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use std::borrow::Cow;
use std::cell::Cell;
use std::{error, fmt};
use std::panic;
use std::path::Path;

use termcolor::{ColorSpec, Color};

Expand Down Expand Up @@ -294,16 +295,9 @@ impl error::Error for ExplicitBug {
pub use diagnostic::{Diagnostic, SubDiagnostic, DiagnosticStyledString, DiagnosticId};
pub use diagnostic_builder::DiagnosticBuilder;

/// 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.
/// A handler deals with errors and other compiler output.
/// Certain errors (fatal, bug, unimpl) may cause immediate exit,
/// others log errors for later reporting.
pub struct Handler {
pub flags: HandlerFlags,

Expand Down Expand Up @@ -775,8 +769,8 @@ impl Handler {
}
}

pub fn maybe_emit_json_directive(&self, directive: String) {
self.emitter.borrow_mut().maybe_emit_json_directive(directive);
pub fn emit_artifact_notification(&self, path: &Path) {
self.emitter.borrow_mut().emit_artifact_notification(path);
}
}

Expand Down
13 changes: 5 additions & 8 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,14 +1048,11 @@ fn encode_and_write_metadata<'tcx>(
tcx.sess.fatal(&format!("couldn't create a temp dir: {}", err))
});
let metadata_filename = emit_metadata(tcx.sess, &metadata, &metadata_tmpdir);
match std::fs::rename(&metadata_filename, &out_filename) {
Ok(_) => {
if tcx.sess.opts.debugging_opts.emit_directives {
tcx.sess.parse_sess.span_diagnostic.maybe_emit_json_directive(
format!("metadata file written: {}", out_filename.display()));
}
}
Err(e) => tcx.sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e)),
if let Err(e) = fs::rename(&metadata_filename, &out_filename) {
tcx.sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e));
}
if tcx.sess.opts.debugging_opts.emit_artifact_notifications {
tcx.sess.parse_sess.span_diagnostic.emit_artifact_notification(&out_filename);
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/libserialize/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,12 +764,18 @@ macro_rules! tuple {

tuple! { T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, }

impl Encodable for path::PathBuf {
impl Encodable for path::Path {
fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
self.to_str().unwrap().encode(e)
}
}

impl Encodable for path::PathBuf {
fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
path::Path::encode(self, e)
}
}

impl Decodable for path::PathBuf {
fn decode<D: Decoder>(d: &mut D) -> Result<path::PathBuf, D::Error> {
let bytes: String = Decodable::decode(d)?;
Expand Down
13 changes: 7 additions & 6 deletions src/libsyntax/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use errors::emitter::{Emitter, HumanReadableErrorType};
use syntax_pos::{MacroBacktrace, Span, SpanLabel, MultiSpan};
use rustc_data_structures::sync::{self, Lrc};
use std::io::{self, Write};
use std::path::Path;
use std::vec;
use std::sync::{Arc, Mutex};

Expand Down Expand Up @@ -91,15 +92,15 @@ impl Emitter for JsonEmitter {
}
}

fn maybe_emit_json_directive(&mut self, directive: String) {
let data = Directive { directive };
fn emit_artifact_notification(&mut self, path: &Path) {
let data = ArtifactNotification { artifact: path };
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);
panic!("failed to print notification: {:?}", e);
}
}
}
Expand Down Expand Up @@ -181,9 +182,9 @@ struct DiagnosticCode {
}

#[derive(RustcEncodable)]
struct Directive {
/// The directive itself.
directive: String,
struct ArtifactNotification<'a> {
/// The path of the artifact.
artifact: &'a Path,
}

impl Diagnostic {
Expand Down
2 changes: 0 additions & 2 deletions src/test/ui/consts/const-eval/unused-broken-const.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
warning: due to multiple output types requested, the explicitly specified output file name will be adapted for each output type

error: any use of this value will cause an error
--> $DIR/unused-broken-const.rs:5:18
|
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/emit-artifact-notifications.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"artifact":"$TEST_BUILD_DIR/emit-artifact-notifications.nll/libemit_artifact_notifications.rmeta"}
6 changes: 6 additions & 0 deletions src/test/ui/emit-artifact-notifications.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// compile-flags:--emit=metadata --error-format=json -Z emit-artifact-notifications
// compile-pass

// A very basic test for the emission of artifact notifications in JSON output.

fn main() {}
1 change: 1 addition & 0 deletions src/test/ui/emit-artifact-notifications.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"artifact":"$TEST_BUILD_DIR/emit-artifact-notifications/libemit_artifact_notifications.rmeta"}
12 changes: 0 additions & 12 deletions src/test/ui/emit-directives.rs

This file was deleted.

1 change: 0 additions & 1 deletion src/test/ui/emit-directives.stderr

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// compile-pass

// FIXME(eddyb) shorten the name so windows doesn't choke on it.
#![crate_name = "trait_test"]

// Regression test related to #56288. Check that a supertrait projection (of
// `Output`) that references `Self` is ok if there is another occurence of
// the same supertrait that specifies the projection explicitly, even if
Expand Down
10 changes: 5 additions & 5 deletions src/tools/compiletest/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use crate::errors::{Error, ErrorKind};
use crate::runtest::ProcRes;
use serde_json;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::str::FromStr;

#[derive(Deserialize)]
Expand All @@ -18,9 +18,9 @@ struct Diagnostic {
}

#[derive(Deserialize)]
struct Directive {
struct ArtifactNotification {
#[allow(dead_code)]
directive: String,
artifact: PathBuf,
}

#[derive(Deserialize, Clone)]
Expand Down Expand Up @@ -75,8 +75,8 @@ pub fn extract_rendered(output: &str) -> String {
if line.starts_with('{') {
if let Ok(diagnostic) = serde_json::from_str::<Diagnostic>(line) {
diagnostic.rendered
} else if let Ok(_directive) = serde_json::from_str::<Directive>(line) {
// Swallow the directive.
} else if let Ok(_) = serde_json::from_str::<ArtifactNotification>(line) {
// Ignore the notification.
None
} else {
print!(
Expand Down
58 changes: 36 additions & 22 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1422,10 +1422,21 @@ impl<'test> TestCx<'test> {
}

fn compile_test(&self) -> ProcRes {
let mut rustc = self.make_compile_args(
&self.testpaths.file,
TargetLocation::ThisFile(self.make_exe_name()),
);
// Only use `make_exe_name` when the test ends up being executed.
let will_execute = match self.config.mode {
RunPass | Ui => self.should_run_successfully(),
Incremental => self.revision.unwrap().starts_with("r"),
RunFail | RunPassValgrind | MirOpt |
DebugInfoBoth | DebugInfoGdb | DebugInfoLldb => true,
_ => false,
};
let output_file = if will_execute {
TargetLocation::ThisFile(self.make_exe_name())
} else {
TargetLocation::ThisDirectory(self.output_base_dir())
};

let mut rustc = self.make_compile_args(&self.testpaths.file, output_file);

rustc.arg("-L").arg(&self.aux_output_dir_name());

Expand Down Expand Up @@ -1882,7 +1893,12 @@ impl<'test> TestCx<'test> {
rustc.arg("-o").arg(path);
}
TargetLocation::ThisDirectory(path) => {
rustc.arg("--out-dir").arg(path);
if is_rustdoc {
// `rustdoc` uses `-o` for the output directory.
rustc.arg("-o").arg(path);
} else {
rustc.arg("--out-dir").arg(path);
}
}
}

Expand Down Expand Up @@ -3138,42 +3154,40 @@ impl<'test> TestCx<'test> {
}

fn normalize_output(&self, output: &str, custom_rules: &[(String, String)]) -> String {
let parent_dir = self.testpaths.file.parent().unwrap();
let cflags = self.props.compile_flags.join(" ");
let json = cflags.contains("--error-format json")
|| cflags.contains("--error-format pretty-json")
|| cflags.contains("--error-format=json")
|| cflags.contains("--error-format=pretty-json");
let parent_dir_str = if json {
parent_dir.display().to_string().replace("\\", "\\\\")
} else {
parent_dir.display().to_string()

let mut normalized = output.to_string();

let mut normalize_path = |from: &Path, to: &str| {
let mut from = from.display().to_string();
if json {
from = from.replace("\\", "\\\\");
}
normalized = normalized.replace(&from, to);
};

let mut normalized = output.replace(&parent_dir_str, "$DIR");
let parent_dir = self.testpaths.file.parent().unwrap();
normalize_path(parent_dir, "$DIR");

// Paths into the libstd/libcore
let src_dir = self.config.src_base.parent().unwrap().parent().unwrap();
let src_dir_str = if json {
src_dir.display().to_string().replace("\\", "\\\\")
} else {
src_dir.display().to_string()
};
normalized = normalized.replace(&src_dir_str, "$SRC_DIR");
normalize_path(src_dir, "$SRC_DIR");

// Paths into the build directory
let test_build_dir = &self.config.build_base;
let parent_build_dir = test_build_dir.parent().unwrap().parent().unwrap().parent().unwrap();

// eg. /home/user/rust/build/x86_64-unknown-linux-gnu/test/ui
normalized = normalized.replace(test_build_dir.to_str().unwrap(), "$TEST_BUILD_DIR");
normalize_path(test_build_dir, "$TEST_BUILD_DIR");
// eg. /home/user/rust/build
normalized = normalized.replace(&parent_build_dir.to_str().unwrap(), "$BUILD_DIR");
normalize_path(parent_build_dir, "$BUILD_DIR");

// Paths into lib directory.
let mut lib_dir = parent_build_dir.parent().unwrap().to_path_buf();
lib_dir.push("lib");
normalized = normalized.replace(&lib_dir.to_str().unwrap(), "$LIB_DIR");
normalize_path(&parent_build_dir.parent().unwrap().join("lib"), "$LIB_DIR");

if json {
// escaped newlines in json strings should be readable
Expand Down

0 comments on commit b8fa4cb

Please sign in to comment.