From 1eaa113581f39d41bc179e300d275cfaab91bd2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Wed, 29 Mar 2017 20:43:01 +0200 Subject: [PATCH 01/17] Emit proper lifetime start intrinsics for personality slots We currently only emit a single call to the lifetime start intrinsic for the personality slot alloca. This happens because we create that call at the time that we create the alloca, instead of creating it each time we start using it. Because LLVM usually removes the alloca before the lifetime intrinsics are even considered, this didn't cause any problems yet, but we should fix this anyway. --- src/librustc_trans/mir/block.rs | 2 +- src/test/codegen/personality_lifetimes.rs | 39 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 src/test/codegen/personality_lifetimes.rs diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index 226d40948c4dc..d69f31a45048d 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -762,7 +762,6 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> { let llretty = Type::struct_(ccx, &[Type::i8p(ccx), Type::i32(ccx)], false); let slot = bcx.alloca(llretty, "personalityslot"); self.llpersonalityslot = Some(slot); - Lifetime::Start.call(bcx, slot); slot } } @@ -794,6 +793,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> { let llretval = bcx.landing_pad(llretty, llpersonality, 1, self.llfn); bcx.set_cleanup(llretval); let slot = self.get_personality_slot(&bcx); + Lifetime::Start.call(&bcx, slot); bcx.store(llretval, slot, None); bcx.br(target_bb); bcx.llbb() diff --git a/src/test/codegen/personality_lifetimes.rs b/src/test/codegen/personality_lifetimes.rs new file mode 100644 index 0000000000000..1d07a2f104082 --- /dev/null +++ b/src/test/codegen/personality_lifetimes.rs @@ -0,0 +1,39 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -O -C no-prepopulate-passes + +#![crate_type="lib"] + +struct S; + +impl Drop for S { + fn drop(&mut self) { + } +} + +fn might_unwind() { +} + +// CHECK-LABEL: @test +#[no_mangle] +pub fn test() { + let _s = S; + // Check that the personality slot alloca gets a lifetime start in each cleanup block, not just + // in the first one. + // CHECK-LABEL: cleanup: + // CHECK: bitcast{{.*}}personalityslot + // CHECK-NEXT: call void @llvm.lifetime.start + // CHECK-LABEL: cleanup1: + // CHECK: bitcast{{.*}}personalityslot + // CHECK-NEXT: call void @llvm.lifetime.start + might_unwind(); + might_unwind(); +} From e1b0027b51d8c8b7558513565c2baa45f1b1b984 Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Thu, 30 Mar 2017 20:49:06 -0600 Subject: [PATCH 02/17] Refer to a subcommand as a subcommand. For some reason 'command' and 'subcommand' were intermixed to mean the same thing. Lets just call it the one thing that it is. --- src/bootstrap/flags.rs | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index b55f3d710ca7b..ea0fc97e22a7b 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -90,12 +90,11 @@ impl Flags { opts.optflag("h", "help", "print this help message"); let usage = |n, opts: &Options| -> ! { - let command = args.get(0).map(|s| &**s); - let brief = format!("Usage: x.py {} [options] [...]", - command.unwrap_or("")); + let subcommand = args.get(0).map(|s| &**s); + let brief = format!("Usage: x.py [options] [...]"); println!("{}", opts.usage(&brief)); - match command { + match subcommand { Some("build") => { println!("\ Arguments: @@ -156,13 +155,13 @@ Arguments: _ => {} } - if let Some(command) = command { - if command == "build" || - command == "dist" || - command == "doc" || - command == "test" || - command == "bench" || - command == "clean" { + if let Some(subcommand) = subcommand { + if subcommand == "build" || + subcommand == "dist" || + subcommand == "doc" || + subcommand == "test" || + subcommand == "bench" || + subcommand == "clean" { println!("Available invocations:"); if args.iter().any(|a| a == "-v") { let flags = Flags::parse(&["build".to_string()]); @@ -170,10 +169,10 @@ Arguments: config.build = flags.build.clone(); let mut build = Build::new(flags, config); metadata::build(&mut build); - step::build_rules(&build).print_help(command); + step::build_rules(&build).print_help(subcommand); } else { println!(" ... elided, run `./x.py {} -h -v` to see", - command); + subcommand); } println!(""); @@ -189,13 +188,13 @@ Subcommands: clean Clean out build directories dist Build and/or install distribution artifacts -To learn more about a subcommand, run `./x.py -h` +To learn more about a subcommand, run `./x.py -h` "); process::exit(n); }; if args.len() == 0 { - println!("a command must be passed"); + println!("a subcommand must be passed"); usage(1, &opts); } let parse = |opts: &Options| { @@ -258,7 +257,7 @@ To learn more about a subcommand, run `./x.py -h` } "--help" => usage(0, &opts), cmd => { - println!("unknown command: {}", cmd); + println!("unknown subcommand: {}", cmd); usage(1, &opts); } }; From 8ad5c95e521f90897a6bd611956d476fcc51c042 Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Thu, 30 Mar 2017 20:58:07 -0600 Subject: [PATCH 03/17] When dealing with the list of all possible subcommands, deal with them in the same order to ease comparing the sections of code in order. I chose the order that appears in the help text, because that is most likely to have been ordered with specific reasoning. --- src/bootstrap/flags.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index ea0fc97e22a7b..556a362a8749a 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -157,11 +157,11 @@ Arguments: if let Some(subcommand) = subcommand { if subcommand == "build" || - subcommand == "dist" || - subcommand == "doc" || subcommand == "test" || subcommand == "bench" || - subcommand == "clean" { + subcommand == "doc" || + subcommand == "clean" || + subcommand == "dist" { println!("Available invocations:"); if args.iter().any(|a| a == "-v") { let flags = Flags::parse(&["build".to_string()]); @@ -219,10 +219,6 @@ To learn more about a subcommand, run `./x.py -h` m = parse(&opts); Subcommand::Build { paths: remaining_as_path(&m) } } - "doc" => { - m = parse(&opts); - Subcommand::Doc { paths: remaining_as_path(&m) } - } "test" => { opts.optmulti("", "test-args", "extra arguments", "ARGS"); m = parse(&opts); @@ -239,6 +235,10 @@ To learn more about a subcommand, run `./x.py -h` test_args: m.opt_strs("test-args"), } } + "doc" => { + m = parse(&opts); + Subcommand::Doc { paths: remaining_as_path(&m) } + } "clean" => { m = parse(&opts); if m.free.len() > 0 { From e1c1e09867e489a41170e726fe64281caaca087a Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Thu, 30 Mar 2017 22:12:01 -0600 Subject: [PATCH 04/17] Don't print build statistics if we explicitly asked for the help message. --- src/bootstrap/bootstrap.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index d5bc6127a1e7f..73f3b1d1cebab 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -593,14 +593,16 @@ def main(): start_time = time() try: bootstrap() - print("Build completed successfully in %s" % format_build_time(time() - start_time)) + if ('-h' not in sys.argv) and ('--help' not in sys.argv): + print("Build completed successfully in %s" % format_build_time(time() - start_time)) except (SystemExit, KeyboardInterrupt) as e: if hasattr(e, 'code') and isinstance(e.code, int): exit_code = e.code else: exit_code = 1 print(e) - print("Build completed unsuccessfully in %s" % format_build_time(time() - start_time)) + if ('-h' not in sys.argv) and ('--help' not in sys.argv): + print("Build completed unsuccessfully in %s" % format_build_time(time() - start_time)) sys.exit(exit_code) if __name__ == '__main__': From 0ba7da344935c553d6c45364e45246729abf6ac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Fri, 31 Mar 2017 20:09:37 +0200 Subject: [PATCH 05/17] Ignore tests for the personality slot lifetimes on MSVC Exception handling on MSVC targets doesn't use personality slots. --- src/test/codegen/personality_lifetimes.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/codegen/personality_lifetimes.rs b/src/test/codegen/personality_lifetimes.rs index 1d07a2f104082..e0de64b26df47 100644 --- a/src/test/codegen/personality_lifetimes.rs +++ b/src/test/codegen/personality_lifetimes.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// ignore-msvc + // compile-flags: -O -C no-prepopulate-passes #![crate_type="lib"] From 584b40578d8ab999031da0855f319a94db06dc47 Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Thu, 30 Mar 2017 22:18:27 -0600 Subject: [PATCH 06/17] Vastly improve the help output. - Don't print 'unknown subcommand' at the top of the help message. The help message now clearly instructs the user to provide a subcommand. - Clarify the usage line. Subcommand is required. Don't echo invalid input back out in the usage line (what the...???). args renamed to paths, because that's what all the args are referred to elsewhere. - List the available subcommands immediately following the usage line. It's the one required argument, after all. - Slightly improve the extra documentation for the build, test, and doc commands. - Don't print 'Available invocations:' at all. It occurred immediately before 'Available paths:'. - Clearly state that running with '-h -v' will produce a list of available paths. --- src/bootstrap/flags.rs | 53 +++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 556a362a8749a..1a260050a9422 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -90,19 +90,31 @@ impl Flags { opts.optflag("h", "help", "print this help message"); let usage = |n, opts: &Options| -> ! { - let subcommand = args.get(0).map(|s| &**s); - let brief = format!("Usage: x.py [options] [...]"); + let subcommand_help = format!("\ +Usage: x.py [options] [...] + +Subcommands: + build Compile either the compiler or libraries + test Build and run some test suites + bench Build and run some benchmarks + doc Build documentation + clean Clean out build directories + dist Build and/or install distribution artifacts - println!("{}", opts.usage(&brief)); +To learn more about a subcommand, run `./x.py -h`"); + + println!("{}", opts.usage(&subcommand_help)); + + let subcommand = args.get(0).map(|s| &**s); match subcommand { Some("build") => { println!("\ Arguments: - This subcommand accepts a number of positional arguments of directories to - the crates and/or artifacts to compile. For example: + This subcommand accepts a number of paths to directories to the crates + and/or artifacts to compile. For example: ./x.py build src/libcore - ./x.py build src/libproc_macro + ./x.py build src/libcore src/libproc_macro ./x.py build src/libstd --stage 1 If no arguments are passed then the complete artifacts for that stage are @@ -120,8 +132,8 @@ Arguments: Some("test") => { println!("\ Arguments: - This subcommand accepts a number of positional arguments of directories to - tests that should be compiled and run. For example: + This subcommand accepts a number of paths to directories to tests that + should be compiled and run. For example: ./x.py test src/test/run-pass ./x.py test src/libstd --test-args hash_map @@ -138,12 +150,12 @@ Arguments: Some("doc") => { println!("\ Arguments: - This subcommand accepts a number of positional arguments of directories of - documentation to build. For example: + This subcommand accepts a number of paths to directories of documentation + to build. For example: ./x.py doc src/doc/book ./x.py doc src/doc/nomicon - ./x.py doc src/libstd + ./x.py doc src/doc/book src/libstd If no arguments are passed then everything is documented: @@ -155,6 +167,7 @@ Arguments: _ => {} } + if let Some(subcommand) = subcommand { if subcommand == "build" || subcommand == "test" || @@ -162,7 +175,6 @@ Arguments: subcommand == "doc" || subcommand == "clean" || subcommand == "dist" { - println!("Available invocations:"); if args.iter().any(|a| a == "-v") { let flags = Flags::parse(&["build".to_string()]); let mut config = Config::default(); @@ -171,7 +183,7 @@ Arguments: metadata::build(&mut build); step::build_rules(&build).print_help(subcommand); } else { - println!(" ... elided, run `./x.py {} -h -v` to see", + println!("Run `./x.py {} -h -v` to see a list of available paths.", subcommand); } @@ -179,18 +191,6 @@ Arguments: } } -println!("\ -Subcommands: - build Compile either the compiler or libraries - test Build and run some test suites - bench Build and run some benchmarks - doc Build documentation - clean Clean out build directories - dist Build and/or install distribution artifacts - -To learn more about a subcommand, run `./x.py -h` -"); - process::exit(n); }; if args.len() == 0 { @@ -256,8 +256,7 @@ To learn more about a subcommand, run `./x.py -h` } } "--help" => usage(0, &opts), - cmd => { - println!("unknown subcommand: {}", cmd); + _ => { usage(1, &opts); } }; From 992a59efc3e3fd976728fc19a1a854afcbb06adf Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Thu, 30 Mar 2017 22:20:51 -0600 Subject: [PATCH 07/17] Using an untyped, one-letter variable binding as an argument to a function and then not using it until over 100 lines later is just mean. --- src/bootstrap/flags.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 1a260050a9422..64b66d9c6bc34 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -89,7 +89,7 @@ impl Flags { opts.optopt("j", "jobs", "number of jobs to run in parallel", "JOBS"); opts.optflag("h", "help", "print this help message"); - let usage = |n, opts: &Options| -> ! { + let usage = |exit_code, opts: &Options| -> ! { let subcommand_help = format!("\ Usage: x.py [options] [...] @@ -191,7 +191,7 @@ Arguments: } } - process::exit(n); + process::exit(exit_code); }; if args.len() == 0 { println!("a subcommand must be passed"); From 5ba579e7f43e607315737899a779d8bd0f378f53 Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Thu, 30 Mar 2017 22:35:55 -0600 Subject: [PATCH 08/17] Save my TODO's as comments, so I don't forget. --- src/bootstrap/flags.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 64b66d9c6bc34..684a39953e2e6 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -212,6 +212,8 @@ Arguments: let remaining_as_path = |m: &Matches| { m.free.iter().map(|p| cwd.join(p)).collect::>() }; + // TODO: Parse subcommand nicely up at top, so options can occur before the subcommand. + // TODO: Get the subcommand-specific options below into the help output let m: Matches; let cmd = match &args[0][..] { From aa4bd0ec0ea0e4edd2a4507c776f9979a05005d1 Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Sat, 1 Apr 2017 15:48:03 -0600 Subject: [PATCH 09/17] Finish the improvements I planned. - No more manual args manipulation -- getopts used for everything. As a result, options can be in any position, now, even before the subcommand. - The additional options for test, bench, and dist now appear in the help output. - No more single-letter variable bindings used internally for large scopes. - Don't output the time measurement when just invoking 'x.py' - Logic is now much more linear. We build strings up, and then print them. --- src/bootstrap/bootstrap.py | 5 +- src/bootstrap/flags.rs | 222 ++++++++++++++++++------------------- src/bootstrap/step.rs | 11 +- 3 files changed, 113 insertions(+), 125 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 73f3b1d1cebab..0e5991a51bc80 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -591,9 +591,10 @@ def bootstrap(): def main(): start_time = time() + help_triggered = ('-h' in sys.argv) or ('--help' in sys.argv) or (len(sys.argv) == 1) try: bootstrap() - if ('-h' not in sys.argv) and ('--help' not in sys.argv): + if not help_triggered: print("Build completed successfully in %s" % format_build_time(time() - start_time)) except (SystemExit, KeyboardInterrupt) as e: if hasattr(e, 'code') and isinstance(e.code, int): @@ -601,7 +602,7 @@ def main(): else: exit_code = 1 print(e) - if ('-h' not in sys.argv) and ('--help' not in sys.argv): + if not help_triggered: print("Build completed unsuccessfully in %s" % format_build_time(time() - start_time)) sys.exit(exit_code) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 684a39953e2e6..e60e279876c37 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -18,7 +18,7 @@ use std::fs; use std::path::PathBuf; use std::process; -use getopts::{Matches, Options}; +use getopts::{Options}; use Build; use config::Config; @@ -75,7 +75,22 @@ pub enum Subcommand { impl Flags { pub fn parse(args: &[String]) -> Flags { + let mut extra_help = String::new(); + let mut subcommand_help = format!("\ +Usage: x.py [options] [...] + +Subcommands: + build Compile either the compiler or libraries + test Build and run some test suites + bench Build and run some benchmarks + doc Build documentation + clean Clean out build directories + dist Build and/or install distribution artifacts + +To learn more about a subcommand, run `./x.py -h`"); + let mut opts = Options::new(); + // Options common to all subcommands opts.optflagmulti("v", "verbose", "use verbose output (-vv for very verbose)"); opts.optflag("i", "incremental", "use incremental compilation"); opts.optopt("", "config", "TOML configuration file for build", "FILE"); @@ -89,26 +104,38 @@ impl Flags { opts.optopt("j", "jobs", "number of jobs to run in parallel", "JOBS"); opts.optflag("h", "help", "print this help message"); - let usage = |exit_code, opts: &Options| -> ! { - let subcommand_help = format!("\ -Usage: x.py [options] [...] - -Subcommands: - build Compile either the compiler or libraries - test Build and run some test suites - bench Build and run some benchmarks - doc Build documentation - clean Clean out build directories - dist Build and/or install distribution artifacts + // fn usage() + let usage = |exit_code: i32, opts: &Options, subcommand_help: &str, extra_help: &str| -> ! { + println!("{}", opts.usage(subcommand_help)); + if !extra_help.is_empty() { + println!("{}", extra_help); + } + process::exit(exit_code); + }; -To learn more about a subcommand, run `./x.py -h`"); + // Get subcommand + let matches = opts.parse(&args[..]).unwrap_or_else(|e| { + // Invalid argument/option format + println!("\n{}\n", e); + usage(1, &opts, &subcommand_help, &extra_help); + }); + let subcommand = match matches.free.get(0) { + Some(s) => { s }, + None => { + // No subcommand -- lets only show the general usage and subcommand help in this case. + println!("{}\n", subcommand_help); + process::exit(0); + } + }; - println!("{}", opts.usage(&subcommand_help)); + // Get any optional paths which occur after the subcommand + let cwd = t!(env::current_dir()); + let paths = matches.free[1..].iter().map(|p| cwd.join(p)).collect::>(); - let subcommand = args.get(0).map(|s| &**s); - match subcommand { - Some("build") => { - println!("\ + // Some subcommands have specific arguments help text + match subcommand.as_str() { + "build" => { + subcommand_help.push_str("\n Arguments: This subcommand accepts a number of paths to directories to the crates and/or artifacts to compile. For example: @@ -125,12 +152,11 @@ Arguments: For a quick build with a usable compile, you can pass: - ./x.py build --stage 1 src/libtest -"); - } - - Some("test") => { - println!("\ + ./x.py build --stage 1 src/libtest"); + } + "test" => { + opts.optmulti("", "test-args", "extra arguments", "ARGS"); + subcommand_help.push_str("\n Arguments: This subcommand accepts a number of paths to directories to tests that should be compiled and run. For example: @@ -143,12 +169,13 @@ Arguments: compiled and tested. ./x.py test - ./x.py test --stage 1 -"); - } - - Some("doc") => { - println!("\ + ./x.py test --stage 1"); + } + "bench" => { + opts.optmulti("", "test-args", "extra arguments", "ARGS"); + } + "doc" => { + subcommand_help.push_str("\n Arguments: This subcommand accepts a number of paths to directories of documentation to build. For example: @@ -160,111 +187,74 @@ Arguments: If no arguments are passed then everything is documented: ./x.py doc - ./x.py doc --stage 1 -"); - } - - _ => {} + ./x.py doc --stage 1"); } - - - if let Some(subcommand) = subcommand { - if subcommand == "build" || - subcommand == "test" || - subcommand == "bench" || - subcommand == "doc" || - subcommand == "clean" || - subcommand == "dist" { - if args.iter().any(|a| a == "-v") { - let flags = Flags::parse(&["build".to_string()]); - let mut config = Config::default(); - config.build = flags.build.clone(); - let mut build = Build::new(flags, config); - metadata::build(&mut build); - step::build_rules(&build).print_help(subcommand); - } else { - println!("Run `./x.py {} -h -v` to see a list of available paths.", - subcommand); - } - - println!(""); - } + "dist" => { + opts.optflag("", "install", "run installer as well"); } - - process::exit(exit_code); + _ => { } }; - if args.len() == 0 { - println!("a subcommand must be passed"); - usage(1, &opts); - } - let parse = |opts: &Options| { - let m = opts.parse(&args[1..]).unwrap_or_else(|e| { - println!("failed to parse options: {}", e); - usage(1, opts); - }); - if m.opt_present("h") { - usage(0, opts); + + // All subcommands can have an optional "Available paths" section + if matches.opt_present("verbose") { + let flags = Flags::parse(&["build".to_string()]); + let mut config = Config::default(); + config.build = flags.build.clone(); + let mut build = Build::new(flags, config); + metadata::build(&mut build); + let maybe_rules_help = step::build_rules(&build).get_help(subcommand); + if maybe_rules_help.is_some() { + extra_help.push_str(maybe_rules_help.unwrap().as_str()); } - return m - }; + } else { + extra_help.push_str(format!("Run `./x.py {} -h -v` to see a list of available paths.", + subcommand).as_str()); + } - let cwd = t!(env::current_dir()); - let remaining_as_path = |m: &Matches| { - m.free.iter().map(|p| cwd.join(p)).collect::>() - }; - // TODO: Parse subcommand nicely up at top, so options can occur before the subcommand. - // TODO: Get the subcommand-specific options below into the help output + // User passed in -h/--help? + if matches.opt_present("help") { + usage(0, &opts, &subcommand_help, &extra_help); + } - let m: Matches; - let cmd = match &args[0][..] { + let cmd = match subcommand.as_str() { "build" => { - m = parse(&opts); - Subcommand::Build { paths: remaining_as_path(&m) } + Subcommand::Build { paths: paths } } "test" => { - opts.optmulti("", "test-args", "extra arguments", "ARGS"); - m = parse(&opts); Subcommand::Test { - paths: remaining_as_path(&m), - test_args: m.opt_strs("test-args"), + paths: paths, + test_args: matches.opt_strs("test-args"), } } "bench" => { - opts.optmulti("", "test-args", "extra arguments", "ARGS"); - m = parse(&opts); Subcommand::Bench { - paths: remaining_as_path(&m), - test_args: m.opt_strs("test-args"), + paths: paths, + test_args: matches.opt_strs("test-args"), } } "doc" => { - m = parse(&opts); - Subcommand::Doc { paths: remaining_as_path(&m) } + Subcommand::Doc { paths: paths } } "clean" => { - m = parse(&opts); - if m.free.len() > 0 { - println!("clean takes no arguments"); - usage(1, &opts); + if matches.free.len() > 0 { + println!("\nclean takes no arguments\n"); + usage(1, &opts, &subcommand_help, &extra_help); } Subcommand::Clean } "dist" => { - opts.optflag("", "install", "run installer as well"); - m = parse(&opts); Subcommand::Dist { - paths: remaining_as_path(&m), - install: m.opt_present("install"), + paths: paths, + install: matches.opt_present("install"), } } - "--help" => usage(0, &opts), _ => { - usage(1, &opts); + usage(1, &opts, &subcommand_help, &extra_help); } }; - let cfg_file = m.opt_str("config").map(PathBuf::from).or_else(|| { + let cfg_file = matches.opt_str("config").map(PathBuf::from).or_else(|| { if fs::metadata("config.toml").is_ok() { Some(PathBuf::from("config.toml")) } else { @@ -272,31 +262,29 @@ Arguments: } }); - let mut stage = m.opt_str("stage").map(|j| j.parse().unwrap()); - - let incremental = m.opt_present("i"); + let mut stage = matches.opt_str("stage").map(|j| j.parse().unwrap()); - if incremental { + if matches.opt_present("incremental") { if stage.is_none() { stage = Some(1); } } Flags { - verbose: m.opt_count("v"), + verbose: matches.opt_count("verbose"), stage: stage, - on_fail: m.opt_str("on-fail"), - keep_stage: m.opt_str("keep-stage").map(|j| j.parse().unwrap()), - build: m.opt_str("build").unwrap_or_else(|| { + on_fail: matches.opt_str("on-fail"), + keep_stage: matches.opt_str("keep-stage").map(|j| j.parse().unwrap()), + build: matches.opt_str("build").unwrap_or_else(|| { env::var("BUILD").unwrap() }), - host: split(m.opt_strs("host")), - target: split(m.opt_strs("target")), + host: split(matches.opt_strs("host")), + target: split(matches.opt_strs("target")), config: cfg_file, - src: m.opt_str("src").map(PathBuf::from), - jobs: m.opt_str("jobs").map(|j| j.parse().unwrap()), + src: matches.opt_str("src").map(PathBuf::from), + jobs: matches.opt_str("jobs").map(|j| j.parse().unwrap()), cmd: cmd, - incremental: incremental, + incremental: matches.opt_present("incremental"), } } } diff --git a/src/bootstrap/step.rs b/src/bootstrap/step.rs index 6eb12fed5abb2..5560b5b0333c8 100644 --- a/src/bootstrap/step.rs +++ b/src/bootstrap/step.rs @@ -978,26 +978,25 @@ invalid rule dependency graph detected, was a rule added and maybe typo'd? } } - pub fn print_help(&self, command: &str) { + pub fn get_help(&self, command: &str) -> Option { let kind = match command { "build" => Kind::Build, "doc" => Kind::Doc, "test" => Kind::Test, "bench" => Kind::Bench, "dist" => Kind::Dist, - _ => return, + _ => return None, }; let rules = self.rules.values().filter(|r| r.kind == kind); let rules = rules.filter(|r| !r.path.contains("nowhere")); let mut rules = rules.collect::>(); rules.sort_by_key(|r| r.path); - println!("Available paths:\n"); + let mut help_string = String::from("Available paths:\n"); for rule in rules { - print!(" ./x.py {} {}", command, rule.path); - - println!(""); + help_string.push_str(format!(" ./x.py {} {}\n", command, rule.path).as_str()); } + Some(help_string) } /// Construct the top-level build steps that we're going to be executing, From 2c9ae48149cb714e7cfd49c038af9b54e261da44 Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Sat, 1 Apr 2017 23:16:46 -0600 Subject: [PATCH 10/17] Oops, we can't parse options until all options have been defined. Tiny bit of manual arg-parsing. Fixed tidy stuff too. --- src/bootstrap/flags.rs | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index e60e279876c37..9b45246bc5002 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -113,31 +113,28 @@ To learn more about a subcommand, run `./x.py -h`"); process::exit(exit_code); }; - // Get subcommand - let matches = opts.parse(&args[..]).unwrap_or_else(|e| { - // Invalid argument/option format - println!("\n{}\n", e); - usage(1, &opts, &subcommand_help, &extra_help); - }); - let subcommand = match matches.free.get(0) { - Some(s) => { s }, - None => { - // No subcommand -- lets only show the general usage and subcommand help in this case. + // We can't use getopt to parse the options until we have completed specifying which + // options are valid, but under the current implementation, some options are conditional on + // the subcommand. Therefore we must manually identify the subcommand first, so that we can + // complete the definition of the options. Then we can use the getopt::Matches object from + // there on out. + let mut possible_subcommands = args.iter().collect::>(); + possible_subcommands.retain(|&s| !s.starts_with('-')); + let subcommand = match possible_subcommands.first() { + Some(s) => s, + None => { + // No subcommand -- show the general usage and subcommand help println!("{}\n", subcommand_help); process::exit(0); } }; - // Get any optional paths which occur after the subcommand - let cwd = t!(env::current_dir()); - let paths = matches.free[1..].iter().map(|p| cwd.join(p)).collect::>(); - // Some subcommands have specific arguments help text match subcommand.as_str() { "build" => { subcommand_help.push_str("\n Arguments: - This subcommand accepts a number of paths to directories to the crates + This subcommand accepts a number of paths to directories to the crates and/or artifacts to compile. For example: ./x.py build src/libcore @@ -195,6 +192,17 @@ Arguments: _ => { } }; + // Done specifying what options are possible, so do the getopts parsing + let matches = opts.parse(&args[..]).unwrap_or_else(|e| { + // Invalid argument/option format + println!("\n{}\n", e); + usage(1, &opts, &subcommand_help, &extra_help); + }); + // Get any optional paths which occur after the subcommand + let cwd = t!(env::current_dir()); + let paths = matches.free[1..].iter().map(|p| cwd.join(p)).collect::>(); + + // All subcommands can have an optional "Available paths" section if matches.opt_present("verbose") { let flags = Flags::parse(&["build".to_string()]); From 6b7258670f606e6951e3ad34c02a598d595dfa6e Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Sun, 2 Apr 2017 10:28:36 -0600 Subject: [PATCH 11/17] Simplify a "use" statement as per @grunweg's feedback. --- src/bootstrap/flags.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 9b45246bc5002..c374d27fe4f3e 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -18,7 +18,7 @@ use std::fs; use std::path::PathBuf; use std::process; -use getopts::{Options}; +use getopts::Options; use Build; use config::Config; From 1e5389853ca3a99a338f3a40cbb96150f711410c Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Sun, 2 Apr 2017 13:11:53 -0600 Subject: [PATCH 12/17] Fix breaking the 'clean' subcommand caused replacing a single-letter variable with the same value in two contexts where it was used differently. That's why you don't use "m" as a variable for hundreds of lines in an outer function, and re-use it in closures several times in the same function. Sheesh. --- src/bootstrap/flags.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index c374d27fe4f3e..2e133664a05b3 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -244,7 +244,7 @@ Arguments: Subcommand::Doc { paths: paths } } "clean" => { - if matches.free.len() > 0 { + if paths.len() > 0 { println!("\nclean takes no arguments\n"); usage(1, &opts, &subcommand_help, &extra_help); } From efd6eab366ad44ac6e9b0b12cd4b9688ab71df6d Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Sun, 2 Apr 2017 16:26:43 -0600 Subject: [PATCH 13/17] Handle symlinks in src/bootstrap/clean.rs (mostly) -- resolves #40860. The broken condition can be replicated with: ``shell export MYARCH=x86_64-apple-darwin && mkdir -p build/$MYARCH/subdir && touch build/$MYARCH/subdir/file && ln -s build/$MYARCH/subdir/file build/$MYARCH/subdir/symlink `` `src/bootstrap/clean.rs` has a custom implementation of removing a tree `fn rm_rf` that used `std::path::Path::{is_file, is_dir, exists}` while recursively deleting directories and files. Unfortunately, `Path`'s implementation of `is_file()` and `is_dir()` and `exists()` always unconditionally follow symlinks, which is the exact opposite of standard implementations of deleting file trees. It appears that this custom implementation is being used to workaround a behavior in Windows where the files often get marked as read-only, which prevents us from simply using something nice and simple like `std::fs::remove_dir_all`, which properly deletes links instead of following them. So it looks like the fix is to use `.symlink_metadata()` to figure out whether tree items are files/symlinks/directories. The one corner case this won't cover is if there is a broken symlink in the "root" `build/$MYARCH` directory, because those initial entries are run through `Path::canonicalize()`, which panics with broken symlinks. So lets just never use symlinks in that one directory. :-) --- src/bootstrap/clean.rs | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/bootstrap/clean.rs b/src/bootstrap/clean.rs index e9547ee42d077..308a0ab3076dd 100644 --- a/src/bootstrap/clean.rs +++ b/src/bootstrap/clean.rs @@ -44,26 +44,25 @@ pub fn clean(build: &Build) { } fn rm_rf(path: &Path) { - if !path.exists() { - return - } - if path.is_file() { - return do_op(path, "remove file", |p| fs::remove_file(p)); - } - - for file in t!(fs::read_dir(path)) { - let file = t!(file).path(); + match path.symlink_metadata() { + Err(e) => { + if e.kind() == ErrorKind::NotFound { + return; + } + panic!("failed to get metadata for file {}: {}", path.display(), e); + }, + Ok(metadata) => { + if metadata.file_type().is_file() || metadata.file_type().is_symlink() { + do_op(path, "remove file", |p| fs::remove_file(p)); + return; + } - if file.is_dir() { - rm_rf(&file); - } else { - // On windows we can't remove a readonly file, and git will - // often clone files as readonly. As a result, we have some - // special logic to remove readonly files on windows. - do_op(&file, "remove file", |p| fs::remove_file(p)); - } - } - do_op(path, "remove dir", |p| fs::remove_dir(p)); + for file in t!(fs::read_dir(path)) { + rm_rf(&t!(file).path()); + } + do_op(path, "remove dir", |p| fs::remove_dir(p)); + }, + }; } fn do_op(path: &Path, desc: &str, mut f: F) @@ -71,9 +70,12 @@ fn do_op(path: &Path, desc: &str, mut f: F) { match f(path) { Ok(()) => {} + // On windows we can't remove a readonly file, and git will often clone files as readonly. + // As a result, we have some special logic to remove readonly files on windows. + // This is also the reason that we can't use things like fs::remove_dir_all(). Err(ref e) if cfg!(windows) && e.kind() == ErrorKind::PermissionDenied => { - let mut p = t!(path.metadata()).permissions(); + let mut p = t!(path.symlink_metadata()).permissions(); p.set_readonly(false); t!(fs::set_permissions(path, p)); f(path).unwrap_or_else(|e| { From 13c744f30d1540f36a6437224d16e3aea0a2ff71 Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Mon, 3 Apr 2017 16:53:04 +0200 Subject: [PATCH 14/17] Move libXtest into libX/tests This change moves: 1. `libcoretest` into `libcore/tests` 2. `libcollectionstest` into `libcollections/tests` This is a follow-up to #39561. --- src/libcollections/Cargo.toml | 4 ++-- .../tests}/binary_heap.rs | 0 src/{libcollectionstest => libcollections/tests}/btree/map.rs | 0 src/{libcollectionstest => libcollections/tests}/btree/mod.rs | 0 src/{libcollectionstest => libcollections/tests}/btree/set.rs | 0 src/{libcollectionstest => libcollections/tests}/cow_str.rs | 0 src/{libcollectionstest => libcollections/tests}/fmt.rs | 0 src/{libcollectionstest => libcollections/tests}/lib.rs | 0 .../tests}/linked_list.rs | 0 src/{libcollectionstest => libcollections/tests}/slice.rs | 0 src/{libcollectionstest => libcollections/tests}/str.rs | 0 src/{libcollectionstest => libcollections/tests}/string.rs | 0 src/{libcollectionstest => libcollections/tests}/vec.rs | 0 src/{libcollectionstest => libcollections/tests}/vec_deque.rs | 0 src/libcore/Cargo.toml | 4 ++-- src/libcore/num/bignum.rs | 2 +- src/libcore/num/diy_float.rs | 2 +- src/libcore/num/flt2dec/mod.rs | 2 +- src/libcore/num/mod.rs | 2 +- src/{libcoretest => libcore/tests}/any.rs | 0 src/{libcoretest => libcore/tests}/array.rs | 0 src/{libcoretest => libcore/tests}/atomic.rs | 0 src/{libcoretest => libcore/tests}/cell.rs | 0 src/{libcoretest => libcore/tests}/char.rs | 0 src/{libcoretest => libcore/tests}/clone.rs | 0 src/{libcoretest => libcore/tests}/cmp.rs | 0 src/{libcoretest => libcore/tests}/fmt/builders.rs | 0 src/{libcoretest => libcore/tests}/fmt/float.rs | 0 src/{libcoretest => libcore/tests}/fmt/mod.rs | 0 src/{libcoretest => libcore/tests}/fmt/num.rs | 0 src/{libcoretest => libcore/tests}/hash/mod.rs | 0 src/{libcoretest => libcore/tests}/hash/sip.rs | 0 src/{libcoretest => libcore/tests}/intrinsics.rs | 0 src/{libcoretest => libcore/tests}/iter.rs | 0 src/{libcoretest => libcore/tests}/lib.rs | 0 src/{libcoretest => libcore/tests}/mem.rs | 0 src/{libcoretest => libcore/tests}/nonzero.rs | 0 src/{libcoretest => libcore/tests}/num/bignum.rs | 0 src/{libcoretest => libcore/tests}/num/dec2flt/mod.rs | 0 src/{libcoretest => libcore/tests}/num/dec2flt/parse.rs | 0 src/{libcoretest => libcore/tests}/num/dec2flt/rawfp.rs | 0 src/{libcoretest => libcore/tests}/num/flt2dec/estimator.rs | 0 src/{libcoretest => libcore/tests}/num/flt2dec/mod.rs | 0 .../tests}/num/flt2dec/strategy/dragon.rs | 0 .../tests}/num/flt2dec/strategy/grisu.rs | 0 src/{libcoretest => libcore/tests}/num/i16.rs | 0 src/{libcoretest => libcore/tests}/num/i32.rs | 0 src/{libcoretest => libcore/tests}/num/i64.rs | 0 src/{libcoretest => libcore/tests}/num/i8.rs | 0 src/{libcoretest => libcore/tests}/num/int_macros.rs | 0 src/{libcoretest => libcore/tests}/num/mod.rs | 0 src/{libcoretest => libcore/tests}/num/u16.rs | 0 src/{libcoretest => libcore/tests}/num/u32.rs | 0 src/{libcoretest => libcore/tests}/num/u64.rs | 0 src/{libcoretest => libcore/tests}/num/u8.rs | 0 src/{libcoretest => libcore/tests}/num/uint_macros.rs | 0 src/{libcoretest => libcore/tests}/ops.rs | 0 src/{libcoretest => libcore/tests}/option.rs | 0 src/{libcoretest => libcore/tests}/ptr.rs | 0 src/{libcoretest => libcore/tests}/result.rs | 0 src/{libcoretest => libcore/tests}/slice.rs | 0 src/{libcoretest => libcore/tests}/str.rs | 2 +- src/{libcoretest => libcore/tests}/tuple.rs | 0 src/tools/tidy/src/pal.rs | 2 +- 64 files changed, 10 insertions(+), 10 deletions(-) rename src/{libcollectionstest => libcollections/tests}/binary_heap.rs (100%) rename src/{libcollectionstest => libcollections/tests}/btree/map.rs (100%) rename src/{libcollectionstest => libcollections/tests}/btree/mod.rs (100%) rename src/{libcollectionstest => libcollections/tests}/btree/set.rs (100%) rename src/{libcollectionstest => libcollections/tests}/cow_str.rs (100%) rename src/{libcollectionstest => libcollections/tests}/fmt.rs (100%) rename src/{libcollectionstest => libcollections/tests}/lib.rs (100%) rename src/{libcollectionstest => libcollections/tests}/linked_list.rs (100%) rename src/{libcollectionstest => libcollections/tests}/slice.rs (100%) rename src/{libcollectionstest => libcollections/tests}/str.rs (100%) rename src/{libcollectionstest => libcollections/tests}/string.rs (100%) rename src/{libcollectionstest => libcollections/tests}/vec.rs (100%) rename src/{libcollectionstest => libcollections/tests}/vec_deque.rs (100%) rename src/{libcoretest => libcore/tests}/any.rs (100%) rename src/{libcoretest => libcore/tests}/array.rs (100%) rename src/{libcoretest => libcore/tests}/atomic.rs (100%) rename src/{libcoretest => libcore/tests}/cell.rs (100%) rename src/{libcoretest => libcore/tests}/char.rs (100%) rename src/{libcoretest => libcore/tests}/clone.rs (100%) rename src/{libcoretest => libcore/tests}/cmp.rs (100%) rename src/{libcoretest => libcore/tests}/fmt/builders.rs (100%) rename src/{libcoretest => libcore/tests}/fmt/float.rs (100%) rename src/{libcoretest => libcore/tests}/fmt/mod.rs (100%) rename src/{libcoretest => libcore/tests}/fmt/num.rs (100%) rename src/{libcoretest => libcore/tests}/hash/mod.rs (100%) rename src/{libcoretest => libcore/tests}/hash/sip.rs (100%) rename src/{libcoretest => libcore/tests}/intrinsics.rs (100%) rename src/{libcoretest => libcore/tests}/iter.rs (100%) rename src/{libcoretest => libcore/tests}/lib.rs (100%) rename src/{libcoretest => libcore/tests}/mem.rs (100%) rename src/{libcoretest => libcore/tests}/nonzero.rs (100%) rename src/{libcoretest => libcore/tests}/num/bignum.rs (100%) rename src/{libcoretest => libcore/tests}/num/dec2flt/mod.rs (100%) rename src/{libcoretest => libcore/tests}/num/dec2flt/parse.rs (100%) rename src/{libcoretest => libcore/tests}/num/dec2flt/rawfp.rs (100%) rename src/{libcoretest => libcore/tests}/num/flt2dec/estimator.rs (100%) rename src/{libcoretest => libcore/tests}/num/flt2dec/mod.rs (100%) rename src/{libcoretest => libcore/tests}/num/flt2dec/strategy/dragon.rs (100%) rename src/{libcoretest => libcore/tests}/num/flt2dec/strategy/grisu.rs (100%) rename src/{libcoretest => libcore/tests}/num/i16.rs (100%) rename src/{libcoretest => libcore/tests}/num/i32.rs (100%) rename src/{libcoretest => libcore/tests}/num/i64.rs (100%) rename src/{libcoretest => libcore/tests}/num/i8.rs (100%) rename src/{libcoretest => libcore/tests}/num/int_macros.rs (100%) rename src/{libcoretest => libcore/tests}/num/mod.rs (100%) rename src/{libcoretest => libcore/tests}/num/u16.rs (100%) rename src/{libcoretest => libcore/tests}/num/u32.rs (100%) rename src/{libcoretest => libcore/tests}/num/u64.rs (100%) rename src/{libcoretest => libcore/tests}/num/u8.rs (100%) rename src/{libcoretest => libcore/tests}/num/uint_macros.rs (100%) rename src/{libcoretest => libcore/tests}/ops.rs (100%) rename src/{libcoretest => libcore/tests}/option.rs (100%) rename src/{libcoretest => libcore/tests}/ptr.rs (100%) rename src/{libcoretest => libcore/tests}/result.rs (100%) rename src/{libcoretest => libcore/tests}/slice.rs (100%) rename src/{libcoretest => libcore/tests}/str.rs (90%) rename src/{libcoretest => libcore/tests}/tuple.rs (100%) diff --git a/src/libcollections/Cargo.toml b/src/libcollections/Cargo.toml index 02b2171a224d0..7e92404bc0d6f 100644 --- a/src/libcollections/Cargo.toml +++ b/src/libcollections/Cargo.toml @@ -13,8 +13,8 @@ core = { path = "../libcore" } std_unicode = { path = "../libstd_unicode" } [[test]] -name = "collectionstest" -path = "../libcollectionstest/lib.rs" +name = "collectionstests" +path = "../libcollections/tests/lib.rs" [[bench]] name = "collectionsbenches" diff --git a/src/libcollectionstest/binary_heap.rs b/src/libcollections/tests/binary_heap.rs similarity index 100% rename from src/libcollectionstest/binary_heap.rs rename to src/libcollections/tests/binary_heap.rs diff --git a/src/libcollectionstest/btree/map.rs b/src/libcollections/tests/btree/map.rs similarity index 100% rename from src/libcollectionstest/btree/map.rs rename to src/libcollections/tests/btree/map.rs diff --git a/src/libcollectionstest/btree/mod.rs b/src/libcollections/tests/btree/mod.rs similarity index 100% rename from src/libcollectionstest/btree/mod.rs rename to src/libcollections/tests/btree/mod.rs diff --git a/src/libcollectionstest/btree/set.rs b/src/libcollections/tests/btree/set.rs similarity index 100% rename from src/libcollectionstest/btree/set.rs rename to src/libcollections/tests/btree/set.rs diff --git a/src/libcollectionstest/cow_str.rs b/src/libcollections/tests/cow_str.rs similarity index 100% rename from src/libcollectionstest/cow_str.rs rename to src/libcollections/tests/cow_str.rs diff --git a/src/libcollectionstest/fmt.rs b/src/libcollections/tests/fmt.rs similarity index 100% rename from src/libcollectionstest/fmt.rs rename to src/libcollections/tests/fmt.rs diff --git a/src/libcollectionstest/lib.rs b/src/libcollections/tests/lib.rs similarity index 100% rename from src/libcollectionstest/lib.rs rename to src/libcollections/tests/lib.rs diff --git a/src/libcollectionstest/linked_list.rs b/src/libcollections/tests/linked_list.rs similarity index 100% rename from src/libcollectionstest/linked_list.rs rename to src/libcollections/tests/linked_list.rs diff --git a/src/libcollectionstest/slice.rs b/src/libcollections/tests/slice.rs similarity index 100% rename from src/libcollectionstest/slice.rs rename to src/libcollections/tests/slice.rs diff --git a/src/libcollectionstest/str.rs b/src/libcollections/tests/str.rs similarity index 100% rename from src/libcollectionstest/str.rs rename to src/libcollections/tests/str.rs diff --git a/src/libcollectionstest/string.rs b/src/libcollections/tests/string.rs similarity index 100% rename from src/libcollectionstest/string.rs rename to src/libcollections/tests/string.rs diff --git a/src/libcollectionstest/vec.rs b/src/libcollections/tests/vec.rs similarity index 100% rename from src/libcollectionstest/vec.rs rename to src/libcollections/tests/vec.rs diff --git a/src/libcollectionstest/vec_deque.rs b/src/libcollections/tests/vec_deque.rs similarity index 100% rename from src/libcollectionstest/vec_deque.rs rename to src/libcollections/tests/vec_deque.rs diff --git a/src/libcore/Cargo.toml b/src/libcore/Cargo.toml index e847c7fa3a0ec..5af63aa970f2c 100644 --- a/src/libcore/Cargo.toml +++ b/src/libcore/Cargo.toml @@ -10,8 +10,8 @@ test = false bench = false [[test]] -name = "coretest" -path = "../libcoretest/lib.rs" +name = "coretests" +path = "../libcore/tests/lib.rs" [[bench]] name = "corebenches" diff --git a/src/libcore/num/bignum.rs b/src/libcore/num/bignum.rs index 8904322ca48f7..b5553fb29475b 100644 --- a/src/libcore/num/bignum.rs +++ b/src/libcore/num/bignum.rs @@ -19,7 +19,7 @@ //! inputs, but we don't do so to avoid the code bloat. Each bignum is still //! tracked for the actual usages, so it normally doesn't matter. -// This module is only for dec2flt and flt2dec, and only public because of libcoretest. +// This module is only for dec2flt and flt2dec, and only public because of coretests. // It is not intended to ever be stabilized. #![doc(hidden)] #![unstable(feature = "core_private_bignum", diff --git a/src/libcore/num/diy_float.rs b/src/libcore/num/diy_float.rs index 11eea753f93f9..6635d95155f4b 100644 --- a/src/libcore/num/diy_float.rs +++ b/src/libcore/num/diy_float.rs @@ -10,7 +10,7 @@ //! Extended precision "soft float", for internal use only. -// This module is only for dec2flt and flt2dec, and only public because of libcoretest. +// This module is only for dec2flt and flt2dec, and only public because of coretests. // It is not intended to ever be stabilized. #![doc(hidden)] #![unstable(feature = "core_private_diy_float", diff --git a/src/libcore/num/flt2dec/mod.rs b/src/libcore/num/flt2dec/mod.rs index f6c03a59f81e4..5123e42df61ca 100644 --- a/src/libcore/num/flt2dec/mod.rs +++ b/src/libcore/num/flt2dec/mod.rs @@ -118,7 +118,7 @@ provide a large enough buffer and `Part` array, and to assemble the final string from resulting `Part`s itself. All algorithms and formatting functions are accompanied by extensive tests -in `coretest::num::flt2dec` module. It also shows how to use individual +in `coretests::num::flt2dec` module. It also shows how to use individual functions. */ diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index df343c9d45f20..f665cfdee77ae 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -90,7 +90,7 @@ impl fmt::UpperHex for Wrapping { mod wrapping; -// All these modules are technically private and only exposed for libcoretest: +// All these modules are technically private and only exposed for coretests: pub mod flt2dec; pub mod dec2flt; pub mod bignum; diff --git a/src/libcoretest/any.rs b/src/libcore/tests/any.rs similarity index 100% rename from src/libcoretest/any.rs rename to src/libcore/tests/any.rs diff --git a/src/libcoretest/array.rs b/src/libcore/tests/array.rs similarity index 100% rename from src/libcoretest/array.rs rename to src/libcore/tests/array.rs diff --git a/src/libcoretest/atomic.rs b/src/libcore/tests/atomic.rs similarity index 100% rename from src/libcoretest/atomic.rs rename to src/libcore/tests/atomic.rs diff --git a/src/libcoretest/cell.rs b/src/libcore/tests/cell.rs similarity index 100% rename from src/libcoretest/cell.rs rename to src/libcore/tests/cell.rs diff --git a/src/libcoretest/char.rs b/src/libcore/tests/char.rs similarity index 100% rename from src/libcoretest/char.rs rename to src/libcore/tests/char.rs diff --git a/src/libcoretest/clone.rs b/src/libcore/tests/clone.rs similarity index 100% rename from src/libcoretest/clone.rs rename to src/libcore/tests/clone.rs diff --git a/src/libcoretest/cmp.rs b/src/libcore/tests/cmp.rs similarity index 100% rename from src/libcoretest/cmp.rs rename to src/libcore/tests/cmp.rs diff --git a/src/libcoretest/fmt/builders.rs b/src/libcore/tests/fmt/builders.rs similarity index 100% rename from src/libcoretest/fmt/builders.rs rename to src/libcore/tests/fmt/builders.rs diff --git a/src/libcoretest/fmt/float.rs b/src/libcore/tests/fmt/float.rs similarity index 100% rename from src/libcoretest/fmt/float.rs rename to src/libcore/tests/fmt/float.rs diff --git a/src/libcoretest/fmt/mod.rs b/src/libcore/tests/fmt/mod.rs similarity index 100% rename from src/libcoretest/fmt/mod.rs rename to src/libcore/tests/fmt/mod.rs diff --git a/src/libcoretest/fmt/num.rs b/src/libcore/tests/fmt/num.rs similarity index 100% rename from src/libcoretest/fmt/num.rs rename to src/libcore/tests/fmt/num.rs diff --git a/src/libcoretest/hash/mod.rs b/src/libcore/tests/hash/mod.rs similarity index 100% rename from src/libcoretest/hash/mod.rs rename to src/libcore/tests/hash/mod.rs diff --git a/src/libcoretest/hash/sip.rs b/src/libcore/tests/hash/sip.rs similarity index 100% rename from src/libcoretest/hash/sip.rs rename to src/libcore/tests/hash/sip.rs diff --git a/src/libcoretest/intrinsics.rs b/src/libcore/tests/intrinsics.rs similarity index 100% rename from src/libcoretest/intrinsics.rs rename to src/libcore/tests/intrinsics.rs diff --git a/src/libcoretest/iter.rs b/src/libcore/tests/iter.rs similarity index 100% rename from src/libcoretest/iter.rs rename to src/libcore/tests/iter.rs diff --git a/src/libcoretest/lib.rs b/src/libcore/tests/lib.rs similarity index 100% rename from src/libcoretest/lib.rs rename to src/libcore/tests/lib.rs diff --git a/src/libcoretest/mem.rs b/src/libcore/tests/mem.rs similarity index 100% rename from src/libcoretest/mem.rs rename to src/libcore/tests/mem.rs diff --git a/src/libcoretest/nonzero.rs b/src/libcore/tests/nonzero.rs similarity index 100% rename from src/libcoretest/nonzero.rs rename to src/libcore/tests/nonzero.rs diff --git a/src/libcoretest/num/bignum.rs b/src/libcore/tests/num/bignum.rs similarity index 100% rename from src/libcoretest/num/bignum.rs rename to src/libcore/tests/num/bignum.rs diff --git a/src/libcoretest/num/dec2flt/mod.rs b/src/libcore/tests/num/dec2flt/mod.rs similarity index 100% rename from src/libcoretest/num/dec2flt/mod.rs rename to src/libcore/tests/num/dec2flt/mod.rs diff --git a/src/libcoretest/num/dec2flt/parse.rs b/src/libcore/tests/num/dec2flt/parse.rs similarity index 100% rename from src/libcoretest/num/dec2flt/parse.rs rename to src/libcore/tests/num/dec2flt/parse.rs diff --git a/src/libcoretest/num/dec2flt/rawfp.rs b/src/libcore/tests/num/dec2flt/rawfp.rs similarity index 100% rename from src/libcoretest/num/dec2flt/rawfp.rs rename to src/libcore/tests/num/dec2flt/rawfp.rs diff --git a/src/libcoretest/num/flt2dec/estimator.rs b/src/libcore/tests/num/flt2dec/estimator.rs similarity index 100% rename from src/libcoretest/num/flt2dec/estimator.rs rename to src/libcore/tests/num/flt2dec/estimator.rs diff --git a/src/libcoretest/num/flt2dec/mod.rs b/src/libcore/tests/num/flt2dec/mod.rs similarity index 100% rename from src/libcoretest/num/flt2dec/mod.rs rename to src/libcore/tests/num/flt2dec/mod.rs diff --git a/src/libcoretest/num/flt2dec/strategy/dragon.rs b/src/libcore/tests/num/flt2dec/strategy/dragon.rs similarity index 100% rename from src/libcoretest/num/flt2dec/strategy/dragon.rs rename to src/libcore/tests/num/flt2dec/strategy/dragon.rs diff --git a/src/libcoretest/num/flt2dec/strategy/grisu.rs b/src/libcore/tests/num/flt2dec/strategy/grisu.rs similarity index 100% rename from src/libcoretest/num/flt2dec/strategy/grisu.rs rename to src/libcore/tests/num/flt2dec/strategy/grisu.rs diff --git a/src/libcoretest/num/i16.rs b/src/libcore/tests/num/i16.rs similarity index 100% rename from src/libcoretest/num/i16.rs rename to src/libcore/tests/num/i16.rs diff --git a/src/libcoretest/num/i32.rs b/src/libcore/tests/num/i32.rs similarity index 100% rename from src/libcoretest/num/i32.rs rename to src/libcore/tests/num/i32.rs diff --git a/src/libcoretest/num/i64.rs b/src/libcore/tests/num/i64.rs similarity index 100% rename from src/libcoretest/num/i64.rs rename to src/libcore/tests/num/i64.rs diff --git a/src/libcoretest/num/i8.rs b/src/libcore/tests/num/i8.rs similarity index 100% rename from src/libcoretest/num/i8.rs rename to src/libcore/tests/num/i8.rs diff --git a/src/libcoretest/num/int_macros.rs b/src/libcore/tests/num/int_macros.rs similarity index 100% rename from src/libcoretest/num/int_macros.rs rename to src/libcore/tests/num/int_macros.rs diff --git a/src/libcoretest/num/mod.rs b/src/libcore/tests/num/mod.rs similarity index 100% rename from src/libcoretest/num/mod.rs rename to src/libcore/tests/num/mod.rs diff --git a/src/libcoretest/num/u16.rs b/src/libcore/tests/num/u16.rs similarity index 100% rename from src/libcoretest/num/u16.rs rename to src/libcore/tests/num/u16.rs diff --git a/src/libcoretest/num/u32.rs b/src/libcore/tests/num/u32.rs similarity index 100% rename from src/libcoretest/num/u32.rs rename to src/libcore/tests/num/u32.rs diff --git a/src/libcoretest/num/u64.rs b/src/libcore/tests/num/u64.rs similarity index 100% rename from src/libcoretest/num/u64.rs rename to src/libcore/tests/num/u64.rs diff --git a/src/libcoretest/num/u8.rs b/src/libcore/tests/num/u8.rs similarity index 100% rename from src/libcoretest/num/u8.rs rename to src/libcore/tests/num/u8.rs diff --git a/src/libcoretest/num/uint_macros.rs b/src/libcore/tests/num/uint_macros.rs similarity index 100% rename from src/libcoretest/num/uint_macros.rs rename to src/libcore/tests/num/uint_macros.rs diff --git a/src/libcoretest/ops.rs b/src/libcore/tests/ops.rs similarity index 100% rename from src/libcoretest/ops.rs rename to src/libcore/tests/ops.rs diff --git a/src/libcoretest/option.rs b/src/libcore/tests/option.rs similarity index 100% rename from src/libcoretest/option.rs rename to src/libcore/tests/option.rs diff --git a/src/libcoretest/ptr.rs b/src/libcore/tests/ptr.rs similarity index 100% rename from src/libcoretest/ptr.rs rename to src/libcore/tests/ptr.rs diff --git a/src/libcoretest/result.rs b/src/libcore/tests/result.rs similarity index 100% rename from src/libcoretest/result.rs rename to src/libcore/tests/result.rs diff --git a/src/libcoretest/slice.rs b/src/libcore/tests/slice.rs similarity index 100% rename from src/libcoretest/slice.rs rename to src/libcore/tests/slice.rs diff --git a/src/libcoretest/str.rs b/src/libcore/tests/str.rs similarity index 90% rename from src/libcoretest/str.rs rename to src/libcore/tests/str.rs index b7d9ba4463d98..08daafccc5404 100644 --- a/src/libcoretest/str.rs +++ b/src/libcore/tests/str.rs @@ -8,4 +8,4 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// All `str` tests live in libcollectiontest::str +// All `str` tests live in collectionstests::str diff --git a/src/libcoretest/tuple.rs b/src/libcore/tests/tuple.rs similarity index 100% rename from src/libcoretest/tuple.rs rename to src/libcore/tests/tuple.rs diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index 3808c05c6b939..0dbf0d4316abd 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -75,7 +75,7 @@ const EXCEPTION_PATHS: &'static [&'static str] = &[ "src/libtest", // Probably should defer to unstable std::sys APIs // std testing crates, ok for now at least - "src/libcoretest", + "src/libcore/tests", // non-std crates "src/test", From 6a9448b523b95dbc850e856508342644fc17db45 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 3 Apr 2017 22:23:32 +0000 Subject: [PATCH 15/17] Fix bug parsing `#[derive]` macro invocations. --- src/librustc_resolve/macros.rs | 6 ++++-- src/libsyntax/ext/derive.rs | 3 ++- src/libsyntax/parse/parser.rs | 20 ++++++++++++++++++++ src/test/run-pass/issue-40962.rs | 20 ++++++++++++++++++++ 4 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 src/test/run-pass/issue-40962.rs diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 05f30f039c8f0..966cb7ee8d8d8 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -222,8 +222,10 @@ impl<'a> base::Resolver for Resolver<'a> { let name = unwrap_or!(attrs[i].name(), continue); if name == "derive" { - let result = attrs[i].parse_list(&self.session.parse_sess, - |parser| parser.parse_path(PathStyle::Mod)); + let result = attrs[i].parse_list(&self.session.parse_sess, |parser| { + parser.parse_path_allowing_meta(PathStyle::Mod) + }); + let mut traits = match result { Ok(traits) => traits, Err(mut e) => { diff --git a/src/libsyntax/ext/derive.rs b/src/libsyntax/ext/derive.rs index c79040424f619..e7c5d8278d977 100644 --- a/src/libsyntax/ext/derive.rs +++ b/src/libsyntax/ext/derive.rs @@ -26,7 +26,8 @@ pub fn collect_derives(cx: &mut ExtCtxt, attrs: &mut Vec) -> Vec return true; } - match attr.parse_list(cx.parse_sess, |parser| parser.parse_path(PathStyle::Mod)) { + match attr.parse_list(cx.parse_sess, + |parser| parser.parse_path_allowing_meta(PathStyle::Mod)) { Ok(ref traits) if traits.is_empty() => { cx.span_warn(attr.span, "empty trait list in `derive`"); false diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index c2c3e5a6855af..a89811d8abb0b 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1754,6 +1754,26 @@ impl<'a> Parser<'a> { }) } + /// Like `parse_path`, but also supports parsing `Word` meta items into paths for back-compat. + /// This is used when parsing derive macro paths in `#[derive]` attributes. + pub fn parse_path_allowing_meta(&mut self, mode: PathStyle) -> PResult<'a, ast::Path> { + let meta_ident = match self.token { + token::Interpolated(ref nt) => match **nt { + token::NtMeta(ref meta) => match meta.node { + ast::MetaItemKind::Word => Some(ast::Ident::with_empty_ctxt(meta.name)), + _ => None, + }, + _ => None, + }, + _ => None, + }; + if let Some(ident) = meta_ident { + self.bump(); + return Ok(ast::Path::from_ident(self.prev_span, ident)); + } + self.parse_path(mode) + } + /// Examples: /// - `a::b::c` /// - `a::b::c(V) -> W` diff --git a/src/test/run-pass/issue-40962.rs b/src/test/run-pass/issue-40962.rs new file mode 100644 index 0000000000000..b35cfa12eab18 --- /dev/null +++ b/src/test/run-pass/issue-40962.rs @@ -0,0 +1,20 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +macro_rules! m { + ($i:meta) => { + #[derive($i)] + struct S; + } +} + +m!(Clone); + +fn main() {} From 20cb7005b0c1e12383a4c2f9031c5c5f5c907efc Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Mon, 3 Apr 2017 19:15:31 -0600 Subject: [PATCH 16/17] Handle options-with-arguments before subcommands such as './x.py -j 10 build' and detect pathological cases like './x.py --option-that-takes-argument clean build' --- src/bootstrap/flags.rs | 60 +++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 2e133664a05b3..3af3b76163c1e 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -119,7 +119,13 @@ To learn more about a subcommand, run `./x.py -h`"); // complete the definition of the options. Then we can use the getopt::Matches object from // there on out. let mut possible_subcommands = args.iter().collect::>(); - possible_subcommands.retain(|&s| !s.starts_with('-')); + possible_subcommands.retain(|&s| + (s == "build") + || (s == "test") + || (s == "bench") + || (s == "doc") + || (s == "clean") + || (s == "dist")); let subcommand = match possible_subcommands.first() { Some(s) => s, None => { @@ -129,7 +135,43 @@ To learn more about a subcommand, run `./x.py -h`"); } }; - // Some subcommands have specific arguments help text + // Some subcommands get extra options + match subcommand.as_str() { + "test" => opts.optmulti("", "test-args", "extra arguments", "ARGS"), + "bench" => opts.optmulti("", "test-args", "extra arguments", "ARGS"), + "dist" => opts.optflag("", "install", "run installer as well"), + _ => { } + }; + + // Done specifying what options are possible, so do the getopts parsing + let matches = opts.parse(&args[..]).unwrap_or_else(|e| { + // Invalid argument/option format + println!("\n{}\n", e); + usage(1, &opts, &subcommand_help, &extra_help); + }); + // Extra sanity check to make sure we didn't hit this crazy corner case: + // + // ./x.py --frobulate clean build + // ^-- option ^ ^- actual subcommand + // \_ arg to option could be mistaken as subcommand + let mut pass_sanity_check = true; + match matches.free.get(0) { + Some(check_subcommand) => { + if &check_subcommand != subcommand { + pass_sanity_check = false; + } + }, + None => { + pass_sanity_check = false; + } + } + if !pass_sanity_check { + println!("{}\n", subcommand_help); + println!("Sorry, I couldn't figure out which subcommand you were trying to specify.\n\ + You may need to move some options to after the subcommand.\n"); + process::exit(1); + } + // Extra help text for some commands match subcommand.as_str() { "build" => { subcommand_help.push_str("\n @@ -152,7 +194,6 @@ Arguments: ./x.py build --stage 1 src/libtest"); } "test" => { - opts.optmulti("", "test-args", "extra arguments", "ARGS"); subcommand_help.push_str("\n Arguments: This subcommand accepts a number of paths to directories to tests that @@ -168,9 +209,6 @@ Arguments: ./x.py test ./x.py test --stage 1"); } - "bench" => { - opts.optmulti("", "test-args", "extra arguments", "ARGS"); - } "doc" => { subcommand_help.push_str("\n Arguments: @@ -186,18 +224,8 @@ Arguments: ./x.py doc ./x.py doc --stage 1"); } - "dist" => { - opts.optflag("", "install", "run installer as well"); - } _ => { } }; - - // Done specifying what options are possible, so do the getopts parsing - let matches = opts.parse(&args[..]).unwrap_or_else(|e| { - // Invalid argument/option format - println!("\n{}\n", e); - usage(1, &opts, &subcommand_help, &extra_help); - }); // Get any optional paths which occur after the subcommand let cwd = t!(env::current_dir()); let paths = matches.free[1..].iter().map(|p| cwd.join(p)).collect::>(); From ea2bfae8694221c92857a0b3dd96f63a8a255db2 Mon Sep 17 00:00:00 2001 From: Nathan Stocks Date: Tue, 4 Apr 2017 13:50:24 -0600 Subject: [PATCH 17/17] Branch arms need to match the return value even if it's not being assigned to anything --- src/bootstrap/flags.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 3af3b76163c1e..a1466d68a135a 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -137,10 +137,10 @@ To learn more about a subcommand, run `./x.py -h`"); // Some subcommands get extra options match subcommand.as_str() { - "test" => opts.optmulti("", "test-args", "extra arguments", "ARGS"), - "bench" => opts.optmulti("", "test-args", "extra arguments", "ARGS"), - "dist" => opts.optflag("", "install", "run installer as well"), - _ => { } + "test" => { opts.optmulti("", "test-args", "extra arguments", "ARGS"); }, + "bench" => { opts.optmulti("", "test-args", "extra arguments", "ARGS"); }, + "dist" => { opts.optflag("", "install", "run installer as well"); }, + _ => { }, }; // Done specifying what options are possible, so do the getopts parsing