1
Fork 0

Address reviewer comments

[breaking-change]

`OptLevel` variants are no longer `pub use`ed by rust::session::config. If you are using these variants, you must change your code to prefix the variant name with `OptLevel`.
This commit is contained in:
Nick Cameron 2016-01-07 09:23:01 +13:00
parent 11dcb48c6a
commit 82f8e5ce84
5 changed files with 69 additions and 68 deletions

View file

@ -72,13 +72,13 @@ pub enum OutputType {
#[derive(Clone, Copy, Debug, PartialEq, Eq)] #[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum ErrorOutputType { pub enum ErrorOutputType {
Tty(ColorConfig), HumanReadable(ColorConfig),
Json, Json,
} }
impl Default for ErrorOutputType { impl Default for ErrorOutputType {
fn default() -> ErrorOutputType { fn default() -> ErrorOutputType {
ErrorOutputType::Tty(ColorConfig::Auto) ErrorOutputType::HumanReadable(ColorConfig::Auto)
} }
} }
@ -135,7 +135,7 @@ pub struct Options {
pub test: bool, pub test: bool,
pub parse_only: bool, pub parse_only: bool,
pub no_trans: bool, pub no_trans: bool,
pub output: ErrorOutputType, pub error_format: ErrorOutputType,
pub treat_err_as_bug: bool, pub treat_err_as_bug: bool,
pub incremental_compilation: bool, pub incremental_compilation: bool,
pub dump_dep_graph: bool, pub dump_dep_graph: bool,
@ -252,12 +252,7 @@ pub fn basic_options() -> Options {
debugging_opts: basic_debugging_options(), debugging_opts: basic_debugging_options(),
prints: Vec::new(), prints: Vec::new(),
cg: basic_codegen_options(), cg: basic_codegen_options(),
<<<<<<< HEAD error_format: ErrorOutputType::default(),
color: ColorConfig::Auto,
=======
output: ErrorOutputType::default(),
show_span: None,
>>>>>>> Add an --output option for specifying an error emitter
externs: HashMap::new(), externs: HashMap::new(),
crate_name: None, crate_name: None,
alt_std_name: None, alt_std_name: None,
@ -324,7 +319,7 @@ macro_rules! options {
$struct_name { $($opt: $init),* } $struct_name { $($opt: $init),* }
} }
pub fn $buildfn(matches: &getopts::Matches, output: ErrorOutputType) -> $struct_name pub fn $buildfn(matches: &getopts::Matches, error_format: ErrorOutputType) -> $struct_name
{ {
let mut op = $defaultfn(); let mut op = $defaultfn();
for option in matches.opt_strs($prefix) { for option in matches.opt_strs($prefix) {
@ -338,20 +333,20 @@ macro_rules! options {
if !setter(&mut op, value) { if !setter(&mut op, value) {
match (value, opt_type_desc) { match (value, opt_type_desc) {
(Some(..), None) => { (Some(..), None) => {
early_error(output, &format!("{} option `{}` takes no \ early_error(error_format, &format!("{} option `{}` takes no \
value", $outputname, key)) value", $outputname, key))
} }
(None, Some(type_desc)) => { (None, Some(type_desc)) => {
early_error(output, &format!("{0} option `{1}` requires \ early_error(error_format, &format!("{0} option `{1}` requires \
{2} ({3} {1}=<value>)", {2} ({3} {1}=<value>)",
$outputname, key, $outputname, key,
type_desc, $prefix)) type_desc, $prefix))
} }
(Some(value), Some(type_desc)) => { (Some(value), Some(type_desc)) => {
early_error(output, &format!("incorrect value `{}` for {} \ early_error(error_format, &format!("incorrect value `{}` for {} \
option `{}` - {} was expected", option `{}` - {} was expected",
value, $outputname, value, $outputname,
key, type_desc)) key, type_desc))
} }
(None, None) => unreachable!() (None, None) => unreachable!()
} }
@ -360,8 +355,8 @@ macro_rules! options {
break; break;
} }
if !found { if !found {
early_error(output, &format!("unknown {} option: `{}`", early_error(error_format, &format!("unknown {} option: `{}`",
$outputname, key)); $outputname, key));
} }
} }
return op; return op;
@ -879,7 +874,7 @@ pub fn rustc_optgroups() -> Vec<RustcOptGroup> {
"NAME=PATH"), "NAME=PATH"),
opt::opt("", "sysroot", "Override the system root", "PATH"), opt::opt("", "sysroot", "Override the system root", "PATH"),
opt::multi("Z", "", "Set internal debugging options", "FLAG"), opt::multi("Z", "", "Set internal debugging options", "FLAG"),
opt::opt_u("", "output", "How errors and other mesasges are produced", "tty|json"), opt::opt_u("", "error-format", "How errors and other messages are produced", "human|json"),
opt::opt("", "color", "Configure coloring of output: opt::opt("", "color", "Configure coloring of output:
auto = colorize, if output goes to a tty (default); auto = colorize, if output goes to a tty (default);
always = always colorize output; always = always colorize output;
@ -929,19 +924,20 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
}; };
// We need the opts_present check because the driver will send us Matches // We need the opts_present check because the driver will send us Matches
// with only stable options if no unstable options are used. Since output is // with only stable options if no unstable options are used. Since error-format
// unstable, it will not be present. We have to use opts_present not // is unstable, it will not be present. We have to use opts_present not
// opt_present because the latter will panic. // opt_present because the latter will panic.
let output = if matches.opts_present(&["output".to_owned()]) { let error_format = if matches.opts_present(&["error-format".to_owned()]) {
match matches.opt_str("output").as_ref().map(|s| &s[..]) { match matches.opt_str("error-format").as_ref().map(|s| &s[..]) {
Some("tty") => ErrorOutputType::Tty(color), Some("human") => ErrorOutputType::HumanReadable(color),
Some("json") => ErrorOutputType::Json, Some("json") => ErrorOutputType::Json,
None => ErrorOutputType::default(), None => ErrorOutputType::default(),
Some(arg) => { Some(arg) => {
early_error(ErrorOutputType::default(), &format!("argument for --output must be \ early_error(ErrorOutputType::default(), &format!("argument for --error-format must \
tty or json (instead was `{}`)", be human or json (instead was \
`{}`)",
arg)) arg))
} }
} }
@ -951,7 +947,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
let unparsed_crate_types = matches.opt_strs("crate-type"); let unparsed_crate_types = matches.opt_strs("crate-type");
let crate_types = parse_crate_types_from_list(unparsed_crate_types) let crate_types = parse_crate_types_from_list(unparsed_crate_types)
.unwrap_or_else(|e| early_error(output, &e[..])); .unwrap_or_else(|e| early_error(error_format, &e[..]));
let mut lint_opts = vec!(); let mut lint_opts = vec!();
let mut describe_lints = false; let mut describe_lints = false;
@ -968,11 +964,11 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
let lint_cap = matches.opt_str("cap-lints").map(|cap| { let lint_cap = matches.opt_str("cap-lints").map(|cap| {
lint::Level::from_str(&cap).unwrap_or_else(|| { lint::Level::from_str(&cap).unwrap_or_else(|| {
early_error(output, &format!("unknown lint level: `{}`", cap)) early_error(error_format, &format!("unknown lint level: `{}`", cap))
}) })
}); });
let debugging_opts = build_debugging_options(matches, output); let debugging_opts = build_debugging_options(matches, error_format);
let parse_only = debugging_opts.parse_only; let parse_only = debugging_opts.parse_only;
let no_trans = debugging_opts.no_trans; let no_trans = debugging_opts.no_trans;
@ -998,7 +994,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
"link" => OutputType::Exe, "link" => OutputType::Exe,
"dep-info" => OutputType::DepInfo, "dep-info" => OutputType::DepInfo,
part => { part => {
early_error(output, &format!("unknown emission type: `{}`", early_error(error_format, &format!("unknown emission type: `{}`",
part)) part))
} }
}; };
@ -1011,7 +1007,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
output_types.insert(OutputType::Exe, None); output_types.insert(OutputType::Exe, None);
} }
let mut cg = build_codegen_options(matches, output); let mut cg = build_codegen_options(matches, error_format);
// Issue #30063: if user requests llvm-related output to one // Issue #30063: if user requests llvm-related output to one
// particular path, disable codegen-units. // particular path, disable codegen-units.
@ -1023,11 +1019,11 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
}).collect(); }).collect();
if !incompatible.is_empty() { if !incompatible.is_empty() {
for ot in &incompatible { for ot in &incompatible {
early_warn(output, &format!("--emit={} with -o incompatible with \ early_warn(error_format, &format!("--emit={} with -o incompatible with \
-C codegen-units=N for N > 1", -C codegen-units=N for N > 1",
ot.shorthand())); ot.shorthand()));
} }
early_warn(output, "resetting to default -C codegen-units=1"); early_warn(error_format, "resetting to default -C codegen-units=1");
cg.codegen_units = 1; cg.codegen_units = 1;
} }
} }
@ -1040,7 +1036,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
let opt_level = { let opt_level = {
if matches.opt_present("O") { if matches.opt_present("O") {
if cg.opt_level.is_some() { if cg.opt_level.is_some() {
early_error(output, "-O and -C opt-level both provided"); early_error(error_format, "-O and -C opt-level both provided");
} }
OptLevel::Default OptLevel::Default
} else { } else {
@ -1051,9 +1047,9 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
Some(2) => OptLevel::Default, Some(2) => OptLevel::Default,
Some(3) => OptLevel::Aggressive, Some(3) => OptLevel::Aggressive,
Some(arg) => { Some(arg) => {
early_error(output, &format!("optimization level needs to be \ early_error(error_format, &format!("optimization level needs to be \
between 0-3 (instead was `{}`)", between 0-3 (instead was `{}`)",
arg)); arg));
} }
} }
} }
@ -1062,7 +1058,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
let gc = debugging_opts.gc; let gc = debugging_opts.gc;
let debuginfo = if matches.opt_present("g") { let debuginfo = if matches.opt_present("g") {
if cg.debuginfo.is_some() { if cg.debuginfo.is_some() {
early_error(output, "-g and -C debuginfo both provided"); early_error(error_format, "-g and -C debuginfo both provided");
} }
FullDebugInfo FullDebugInfo
} else { } else {
@ -1071,16 +1067,16 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
Some(1) => LimitedDebugInfo, Some(1) => LimitedDebugInfo,
Some(2) => FullDebugInfo, Some(2) => FullDebugInfo,
Some(arg) => { Some(arg) => {
early_error(output, &format!("debug info level needs to be between \ early_error(error_format, &format!("debug info level needs to be between \
0-2 (instead was `{}`)", 0-2 (instead was `{}`)",
arg)); arg));
} }
} }
}; };
let mut search_paths = SearchPaths::new(); let mut search_paths = SearchPaths::new();
for s in &matches.opt_strs("L") { for s in &matches.opt_strs("L") {
search_paths.add_path(&s[..], output); search_paths.add_path(&s[..], error_format);
} }
let libs = matches.opt_strs("l").into_iter().map(|s| { let libs = matches.opt_strs("l").into_iter().map(|s| {
@ -1092,9 +1088,9 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
(Some(name), "framework") => (name, cstore::NativeFramework), (Some(name), "framework") => (name, cstore::NativeFramework),
(Some(name), "static") => (name, cstore::NativeStatic), (Some(name), "static") => (name, cstore::NativeStatic),
(_, s) => { (_, s) => {
early_error(output, &format!("unknown library kind `{}`, expected \ early_error(error_format, &format!("unknown library kind `{}`, expected \
one of dylib, framework, or static", one of dylib, framework, or static",
s)); s));
} }
}; };
(name.to_string(), kind) (name.to_string(), kind)
@ -1109,14 +1105,14 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
"file-names" => PrintRequest::FileNames, "file-names" => PrintRequest::FileNames,
"sysroot" => PrintRequest::Sysroot, "sysroot" => PrintRequest::Sysroot,
req => { req => {
early_error(output, &format!("unknown print request `{}`", req)) early_error(error_format, &format!("unknown print request `{}`", req))
} }
} }
}).collect::<Vec<_>>(); }).collect::<Vec<_>>();
if !cg.remark.is_empty() && debuginfo == NoDebugInfo { if !cg.remark.is_empty() && debuginfo == NoDebugInfo {
early_warn(output, "-C remark will not show source locations without \ early_warn(error_format, "-C remark will not show source locations without \
--debuginfo"); --debuginfo");
} }
let mut externs = HashMap::new(); let mut externs = HashMap::new();
@ -1124,11 +1120,11 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
let mut parts = arg.splitn(2, '='); let mut parts = arg.splitn(2, '=');
let name = match parts.next() { let name = match parts.next() {
Some(s) => s, Some(s) => s,
None => early_error(output, "--extern value must not be empty"), None => early_error(error_format, "--extern value must not be empty"),
}; };
let location = match parts.next() { let location = match parts.next() {
Some(s) => s, Some(s) => s,
None => early_error(output, "--extern value must be of the format `foo=bar`"), None => early_error(error_format, "--extern value must be of the format `foo=bar`"),
}; };
externs.entry(name.to_string()).or_insert(vec![]).push(location.to_string()); externs.entry(name.to_string()).or_insert(vec![]).push(location.to_string());
@ -1159,7 +1155,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
debugging_opts: debugging_opts, debugging_opts: debugging_opts,
prints: prints, prints: prints,
cg: cg, cg: cg,
output: output, error_format: error_format,
externs: externs, externs: externs,
crate_name: crate_name, crate_name: crate_name,
alt_std_name: None, alt_std_name: None,

View file

@ -406,8 +406,8 @@ pub fn build_session(sopts: config::Options,
let treat_err_as_bug = sopts.treat_err_as_bug; let treat_err_as_bug = sopts.treat_err_as_bug;
let codemap = Rc::new(codemap::CodeMap::new()); let codemap = Rc::new(codemap::CodeMap::new());
let emitter: Box<Emitter> = match sopts.output { let emitter: Box<Emitter> = match sopts.error_format {
config::ErrorOutputType::Tty(color_config) => { config::ErrorOutputType::HumanReadable(color_config) => {
Box::new(EmitterWriter::stderr(color_config, Some(registry), codemap.clone())) Box::new(EmitterWriter::stderr(color_config, Some(registry), codemap.clone()))
} }
config::ErrorOutputType::Json => { config::ErrorOutputType::Json => {
@ -483,7 +483,9 @@ pub fn build_session_(sopts: config::Options,
pub fn early_error(output: config::ErrorOutputType, msg: &str) -> ! { pub fn early_error(output: config::ErrorOutputType, msg: &str) -> ! {
let mut emitter: Box<Emitter> = match output { let mut emitter: Box<Emitter> = match output {
config::ErrorOutputType::Tty(color_config) => Box::new(BasicEmitter::stderr(color_config)), config::ErrorOutputType::HumanReadable(color_config) => {
Box::new(BasicEmitter::stderr(color_config))
}
config::ErrorOutputType::Json => Box::new(JsonEmitter::basic()), config::ErrorOutputType::Json => Box::new(JsonEmitter::basic()),
}; };
emitter.emit(None, msg, None, errors::Level::Fatal); emitter.emit(None, msg, None, errors::Level::Fatal);
@ -492,7 +494,9 @@ pub fn early_error(output: config::ErrorOutputType, msg: &str) -> ! {
pub fn early_warn(output: config::ErrorOutputType, msg: &str) { pub fn early_warn(output: config::ErrorOutputType, msg: &str) {
let mut emitter: Box<Emitter> = match output { let mut emitter: Box<Emitter> = match output {
config::ErrorOutputType::Tty(color_config) => Box::new(BasicEmitter::stderr(color_config)), config::ErrorOutputType::HumanReadable(color_config) => {
Box::new(BasicEmitter::stderr(color_config))
}
config::ErrorOutputType::Json => Box::new(JsonEmitter::basic()), config::ErrorOutputType::Json => Box::new(JsonEmitter::basic()),
}; };
emitter.emit(None, msg, None, errors::Level::Warning); emitter.emit(None, msg, None, errors::Level::Warning);

View file

@ -127,7 +127,7 @@ pub fn run_compiler<'a>(args: &[String], callbacks: &mut CompilerCalls<'a>) {
let descriptions = diagnostics_registry(); let descriptions = diagnostics_registry();
do_or_return!(callbacks.early_callback(&matches, &descriptions, sopts.output)); do_or_return!(callbacks.early_callback(&matches, &descriptions, sopts.error_format));
let (odir, ofile) = make_output(&matches); let (odir, ofile) = make_output(&matches);
let (input, input_file_path) = match make_input(&matches.free) { let (input, input_file_path) = match make_input(&matches.free) {
@ -340,10 +340,10 @@ impl<'a> CompilerCalls<'a> for RustcDefaultCalls {
if should_stop == Compilation::Stop { if should_stop == Compilation::Stop {
return None; return None;
} }
early_error(sopts.output, "no input filename given"); early_error(sopts.error_format, "no input filename given");
} }
1 => panic!("make_input should have provided valid inputs"), 1 => panic!("make_input should have provided valid inputs"),
_ => early_error(sopts.output, "multiple input filenames provided"), _ => early_error(sopts.error_format, "multiple input filenames provided"),
} }
None None

View file

@ -3,5 +3,6 @@
all: all:
cp foo.rs $(TMPDIR) cp foo.rs $(TMPDIR)
cd $(TMPDIR) cd $(TMPDIR)
$(RUSTC) -Z unstable-options --output=json foo.rs 2>foo.log || true -$(RUSTC) -Z unstable-options --error-format=json foo.rs 2>foo.log
grep -q '{"message":"unresolved name `y`","code":{"code":"E0425","explanation":"\\nAn unresolved name was used. Example of erroneous codes.*"},"level":"error","span":{"file_name":"foo.rs","byte_start":523,"byte_end":524,"line_start":14,"line_end":14,"column_start":18,"column_end":19},"children":\[\]}' foo.log grep -q '{"message":"unresolved name `y`","code":{"code":"E0425","explanation":"\\nAn unresolved name was used. Example of erroneous codes.*"},"level":"error","span":{"file_name":"foo.rs","byte_start":496,"byte_end":497,"line_start":12,"line_end":12,"column_start":18,"column_end":19},"children":\[\]}' foo.log
grep -q '{"message":".*","code":{"code":"E0277","explanation":"\\nYou tried.*"},"level":"error","span":{.*},"children":\[{"message":"the .*","code":null,"level":"help","span":{"file_name":"foo.rs","byte_start":504,"byte_end":516,"line_start":14,"line_end":14,"column_start":0,"column_end":0},"children":\[\]},{"message":" <u8 as core::ops::Add>","code":null,"level":"help",' foo.log

View file

@ -8,8 +8,8 @@
// option. This file may not be copied, modified, or distributed // option. This file may not be copied, modified, or distributed
// except according to those terms. // except according to those terms.
// ignore-tidy-linelength
fn main() { fn main() {
let x = 42 + y; let x = 42 + y;
42u8 + 42i32;
} }