From 442bef754eb9a521ecd7047b00ab796a54d8b10c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 7 Apr 2019 11:16:26 +0200 Subject: [PATCH 01/23] compiletest normalization: preserve non-JSON lines such as ICEs --- src/tools/compiletest/src/json.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/json.rs b/src/tools/compiletest/src/json.rs index 15d449260efa0..ba919f4c4118a 100644 --- a/src/tools/compiletest/src/json.rs +++ b/src/tools/compiletest/src/json.rs @@ -78,7 +78,8 @@ pub fn extract_rendered(output: &str, proc_res: &ProcRes) -> String { } } } else { - None + // preserve non-JSON lines, such as ICEs + Some(format!("{}\n", line)) } }) .collect() From 0c455463ba4140aaeb4179c2eee3644863d17322 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 7 Apr 2019 12:43:40 +0200 Subject: [PATCH 02/23] fix tests --- src/librustc_mir/interpret/validity.rs | 2 +- .../cfg-arg-invalid-1.stderr | 2 ++ .../cfg-arg-invalid-2.stderr | 2 ++ .../cfg-arg-invalid-3.stderr | 2 ++ .../cfg-arg-invalid-4.stderr | 2 ++ .../cfg-arg-invalid-5.stderr | 2 ++ .../cfg-empty-codemap.stderr | 2 ++ src/test/ui/pattern/const-pat-ice.stderr | 13 +++++++++++++ 8 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/conditional-compilation/cfg-arg-invalid-1.stderr create mode 100644 src/test/ui/conditional-compilation/cfg-arg-invalid-2.stderr create mode 100644 src/test/ui/conditional-compilation/cfg-arg-invalid-3.stderr create mode 100644 src/test/ui/conditional-compilation/cfg-arg-invalid-4.stderr create mode 100644 src/test/ui/conditional-compilation/cfg-arg-invalid-5.stderr create mode 100644 src/test/ui/conditional-compilation/cfg-empty-codemap.stderr create mode 100644 src/test/ui/pattern/const-pat-ice.stderr diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index d4cf906619d89..11e4ffe19da62 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -353,7 +353,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> match self.ecx.memory.check_align(ptr, align) { Ok(_) => {}, Err(err) => { - error!("{:?} is not aligned to {:?}", ptr, align); + info!("{:?} is not aligned to {:?}", ptr, align); match err.kind { InterpError::InvalidNullPointerUsage => return validation_failure!("NULL reference", self.path), diff --git a/src/test/ui/conditional-compilation/cfg-arg-invalid-1.stderr b/src/test/ui/conditional-compilation/cfg-arg-invalid-1.stderr new file mode 100644 index 0000000000000..1e7922a9ff155 --- /dev/null +++ b/src/test/ui/conditional-compilation/cfg-arg-invalid-1.stderr @@ -0,0 +1,2 @@ +error: invalid `--cfg` argument: `a(b=c)` (expected `key` or `key="value"`) + diff --git a/src/test/ui/conditional-compilation/cfg-arg-invalid-2.stderr b/src/test/ui/conditional-compilation/cfg-arg-invalid-2.stderr new file mode 100644 index 0000000000000..b92e1fd3d97cf --- /dev/null +++ b/src/test/ui/conditional-compilation/cfg-arg-invalid-2.stderr @@ -0,0 +1,2 @@ +error: invalid `--cfg` argument: `a{b}` (expected `key` or `key="value"`) + diff --git a/src/test/ui/conditional-compilation/cfg-arg-invalid-3.stderr b/src/test/ui/conditional-compilation/cfg-arg-invalid-3.stderr new file mode 100644 index 0000000000000..5412f7ffd5c9c --- /dev/null +++ b/src/test/ui/conditional-compilation/cfg-arg-invalid-3.stderr @@ -0,0 +1,2 @@ +error: invalid `--cfg` argument: `a::b` (argument key must be an identifier) + diff --git a/src/test/ui/conditional-compilation/cfg-arg-invalid-4.stderr b/src/test/ui/conditional-compilation/cfg-arg-invalid-4.stderr new file mode 100644 index 0000000000000..6853a69b9eb22 --- /dev/null +++ b/src/test/ui/conditional-compilation/cfg-arg-invalid-4.stderr @@ -0,0 +1,2 @@ +error: invalid `--cfg` argument: `a(b)` (expected `key` or `key="value"`) + diff --git a/src/test/ui/conditional-compilation/cfg-arg-invalid-5.stderr b/src/test/ui/conditional-compilation/cfg-arg-invalid-5.stderr new file mode 100644 index 0000000000000..aafc4e8980cb5 --- /dev/null +++ b/src/test/ui/conditional-compilation/cfg-arg-invalid-5.stderr @@ -0,0 +1,2 @@ +error: invalid `--cfg` argument: `a=10` (argument value must be a string) + diff --git a/src/test/ui/conditional-compilation/cfg-empty-codemap.stderr b/src/test/ui/conditional-compilation/cfg-empty-codemap.stderr new file mode 100644 index 0000000000000..128e3cd730680 --- /dev/null +++ b/src/test/ui/conditional-compilation/cfg-empty-codemap.stderr @@ -0,0 +1,2 @@ +error: invalid `--cfg` argument: `""` (expected `key` or `key="value"`) + diff --git a/src/test/ui/pattern/const-pat-ice.stderr b/src/test/ui/pattern/const-pat-ice.stderr new file mode 100644 index 0000000000000..11cb507d8e6d7 --- /dev/null +++ b/src/test/ui/pattern/const-pat-ice.stderr @@ -0,0 +1,13 @@ +thread 'rustc' panicked at 'assertion failed: rows.iter().all(|r| r.len() == v.len())', src/librustc_mir/hair/pattern/_match.rs:1069:5 +note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace. + +error: internal compiler error: unexpected panic + +note: the compiler unexpectedly panicked. this is a bug. + +note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports + +note: rustc 1.35.0-dev running on x86_64-unknown-linux-gnu + +note: compiler flags: -Z threads=1 -Z ui-testing -Z unstable-options -C prefer-dynamic -C rpath + From 83dcc968fb084bf9c4e1658ea3b864167793717c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 7 Apr 2019 16:40:27 +0200 Subject: [PATCH 03/23] fix output test for backtrace-debuginfo.rs --- src/test/run-pass/backtrace-debuginfo.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/run-pass/backtrace-debuginfo.rs b/src/test/run-pass/backtrace-debuginfo.rs index 37c889e9df4ad..7f038ddecc1fa 100644 --- a/src/test/run-pass/backtrace-debuginfo.rs +++ b/src/test/run-pass/backtrace-debuginfo.rs @@ -10,9 +10,11 @@ // ignore-pretty issue #37195 // ignore-cloudabi spawning processes is not supported // ignore-emscripten spawning processes is not supported +// normalize-stderr-test ".*\n" -> "" -// note that above `-opt-bisect-limit=0` is used to basically disable -// optimizations +// Note that above `-opt-bisect-limit=0` is used to basically disable +// optimizations. It creates tons of output on stderr, hence we normalize +// that away entirely. use std::env; From be83bd5c107e1efc5c16f72441bf08757e160049 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 7 Apr 2019 16:45:28 +0200 Subject: [PATCH 04/23] normalize flags and rustc version in ICE repro --- src/test/ui/pattern/const-pat-ice.rs | 2 ++ src/test/ui/pattern/const-pat-ice.stderr | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/ui/pattern/const-pat-ice.rs b/src/test/ui/pattern/const-pat-ice.rs index 865c54be1ad7b..b866c7fa6cc9e 100644 --- a/src/test/ui/pattern/const-pat-ice.rs +++ b/src/test/ui/pattern/const-pat-ice.rs @@ -1,5 +1,7 @@ // failure-status: 101 // rustc-env:RUST_BACKTRACE=0 +// normalize-stderr-test "note: rustc 1.* running on .*" -> "note: rustc VERSION running on TARGET" +// normalize-stderr-test "note: compiler flags: .*" -> "note: compiler flags: FLAGS" // This is a repro test for an ICE in our pattern handling of constants. diff --git a/src/test/ui/pattern/const-pat-ice.stderr b/src/test/ui/pattern/const-pat-ice.stderr index 11cb507d8e6d7..03580dfecfb59 100644 --- a/src/test/ui/pattern/const-pat-ice.stderr +++ b/src/test/ui/pattern/const-pat-ice.stderr @@ -7,7 +7,7 @@ note: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports -note: rustc 1.35.0-dev running on x86_64-unknown-linux-gnu +note: rustc VERSION running on TARGET -note: compiler flags: -Z threads=1 -Z ui-testing -Z unstable-options -C prefer-dynamic -C rpath +note: compiler flags: FLAGS From 126ac9ef6c11ced54ba9191aba916dd7d5206159 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 7 Apr 2019 17:56:50 +0200 Subject: [PATCH 05/23] Add test with current behaviour. This commit adds a test demonstrating the current behaviour when a macro defined in a module with the `#[macro_export]` is imported from the module rather than the crate root. --- src/test/ui/auxiliary/issue-59764.rs | 8 ++++++++ src/test/ui/issue-59764.rs | 14 ++++++++++++++ src/test/ui/issue-59764.stderr | 24 ++++++++++++++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 src/test/ui/auxiliary/issue-59764.rs create mode 100644 src/test/ui/issue-59764.rs create mode 100644 src/test/ui/issue-59764.stderr diff --git a/src/test/ui/auxiliary/issue-59764.rs b/src/test/ui/auxiliary/issue-59764.rs new file mode 100644 index 0000000000000..0243ef5c6c74a --- /dev/null +++ b/src/test/ui/auxiliary/issue-59764.rs @@ -0,0 +1,8 @@ +pub mod foo { + #[macro_export] + macro_rules! makro { + ($foo:ident) => { + fn $foo() { } + } + } +} diff --git a/src/test/ui/issue-59764.rs b/src/test/ui/issue-59764.rs new file mode 100644 index 0000000000000..8ac9b9eaba181 --- /dev/null +++ b/src/test/ui/issue-59764.rs @@ -0,0 +1,14 @@ +// aux-build:issue-59764.rs +// compile-flags:--extern issue_59764 +// edition:2018 + +use issue_59764::foo::makro; +//~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] + +makro!(bar); +//~^ ERROR cannot determine resolution for the macro `makro` + +fn main() { + bar(); + //~^ ERROR cannot find function `bar` in this scope [E0425] +} diff --git a/src/test/ui/issue-59764.stderr b/src/test/ui/issue-59764.stderr new file mode 100644 index 0000000000000..a428488b009ec --- /dev/null +++ b/src/test/ui/issue-59764.stderr @@ -0,0 +1,24 @@ +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:5:5 + | +LL | use issue_59764::foo::makro; + | ^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo` + +error: cannot determine resolution for the macro `makro` + --> $DIR/issue-59764.rs:8:1 + | +LL | makro!(bar); + | ^^^^^ + | + = note: import resolution is stuck, try simplifying macro imports + +error[E0425]: cannot find function `bar` in this scope + --> $DIR/issue-59764.rs:12:5 + | +LL | bar(); + | ^^^ not found in this scope + +error: aborting due to 3 previous errors + +Some errors occurred: E0425, E0432. +For more information about an error, try `rustc --explain E0425`. From 724ca0584e2be0714edf2f30b86230666d7a9d19 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 9 Apr 2019 14:47:00 +0200 Subject: [PATCH 06/23] Exclude profiler-generated symbols from MSVC __imp_-symbol workaround. --- src/librustc_codegen_llvm/back/write.rs | 18 +++++++++++++++++- .../pgo-gen-no-imp-symbols/Makefile | 11 +++++++++++ .../pgo-gen-no-imp-symbols/test.rs | 1 + 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/Makefile create mode 100644 src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/test.rs diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index f0ed201ad5c27..3628adeb3e9a7 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -795,6 +795,7 @@ fn create_msvc_imps( } else { "\x01__imp_" }; + unsafe { let i8p_ty = Type::i8p_llcx(llcx); let globals = base::iter_globals(llmod) @@ -802,14 +803,23 @@ fn create_msvc_imps( llvm::LLVMRustGetLinkage(val) == llvm::Linkage::ExternalLinkage && llvm::LLVMIsDeclaration(val) == 0 }) - .map(move |val| { + .filter_map(|val| { + // Exclude some symbols that we know are not Rust symbols. let name = CStr::from_ptr(llvm::LLVMGetValueName(val)); + if ignored(name.to_bytes()) { + None + } else { + Some((val, name)) + } + }) + .map(move |(val, name)| { let mut imp_name = prefix.as_bytes().to_vec(); imp_name.extend(name.to_bytes()); let imp_name = CString::new(imp_name).unwrap(); (imp_name, val) }) .collect::>(); + for (imp_name, val) in globals { let imp = llvm::LLVMAddGlobal(llmod, i8p_ty, @@ -818,4 +828,10 @@ fn create_msvc_imps( llvm::LLVMRustSetLinkage(imp, llvm::Linkage::ExternalLinkage); } } + + // Use this function to exclude certain symbols from `__imp` generation. + fn ignored(symbol_name: &[u8]) -> bool { + // These are symbols generated by LLVM's profiling instrumentation + symbol_name.starts_with(b"__llvm_profile_") + } } diff --git a/src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/Makefile b/src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/Makefile new file mode 100644 index 0000000000000..dc52e91317a5a --- /dev/null +++ b/src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/Makefile @@ -0,0 +1,11 @@ +-include ../tools.mk + +all: +ifeq ($(PROFILER_SUPPORT),1) + $(RUSTC) -O -Ccodegen-units=1 -Z pgo-gen="$(TMPDIR)/test.profraw" --emit=llvm-ir test.rs + # We expect symbols starting with "__llvm_profile_". + $(CGREP) "__llvm_profile_" < $(TMPDIR)/test.ll + # We do NOT expect the "__imp_" version of these symbols. + $(CGREP) -v "__imp___llvm_profile_" < $(TMPDIR)/test.ll # 64 bit + $(CGREP) -v "__imp____llvm_profile_" < $(TMPDIR)/test.ll # 32 bit +endif diff --git a/src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/test.rs b/src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/test.rs new file mode 100644 index 0000000000000..f328e4d9d04c3 --- /dev/null +++ b/src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/test.rs @@ -0,0 +1 @@ +fn main() {} From 3a35b7e6c711a700efa591b0ef4a20ac864913e7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 9 Apr 2019 22:08:57 +0200 Subject: [PATCH 07/23] normalize away spurious error --- src/test/ui/duplicate/dupe-symbols-7.rs | 4 ++++ src/test/ui/duplicate/dupe-symbols-7.stderr | 2 +- src/test/ui/huge-array.rs | 4 ++++ src/test/ui/issues/issue-15919.rs | 4 ++++ src/test/ui/issues/issue-17913.rs | 4 ++++ src/test/ui/linkage2.rs | 4 ++++ src/test/ui/linkage2.stderr | 2 +- src/test/ui/linkage3.rs | 4 ++++ src/test/ui/linkage3.stderr | 2 +- 9 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/test/ui/duplicate/dupe-symbols-7.rs b/src/test/ui/duplicate/dupe-symbols-7.rs index b28f1a40fe93b..b838aaa102ebb 100644 --- a/src/test/ui/duplicate/dupe-symbols-7.rs +++ b/src/test/ui/duplicate/dupe-symbols-7.rs @@ -1,5 +1,9 @@ // // error-pattern: entry symbol `main` defined multiple times + +// FIXME https://github.com/rust-lang/rust/issues/59774 +// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> "" +// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> "" #![allow(warnings)] #[no_mangle] diff --git a/src/test/ui/duplicate/dupe-symbols-7.stderr b/src/test/ui/duplicate/dupe-symbols-7.stderr index a1dbbb0c685fb..7d033ec3d85fa 100644 --- a/src/test/ui/duplicate/dupe-symbols-7.stderr +++ b/src/test/ui/duplicate/dupe-symbols-7.stderr @@ -1,5 +1,5 @@ error: entry symbol `main` defined multiple times - --> $DIR/dupe-symbols-7.rs:6:1 + --> $DIR/dupe-symbols-7.rs:10:1 | LL | fn main(){} | ^^^^^^^^^^^ diff --git a/src/test/ui/huge-array.rs b/src/test/ui/huge-array.rs index 0608b23dac6b1..f58dcd5806761 100644 --- a/src/test/ui/huge-array.rs +++ b/src/test/ui/huge-array.rs @@ -1,5 +1,9 @@ // error-pattern:; 1518600000 +// FIXME https://github.com/rust-lang/rust/issues/59774 +// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> "" +// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> "" + fn generic(t: T) { let s: [T; 1518600000] = [t; 1518600000]; } diff --git a/src/test/ui/issues/issue-15919.rs b/src/test/ui/issues/issue-15919.rs index 19ecf2f657e0a..a7ac4802a12d5 100644 --- a/src/test/ui/issues/issue-15919.rs +++ b/src/test/ui/issues/issue-15919.rs @@ -1,6 +1,10 @@ // error-pattern: too big for the current architecture // normalize-stderr-test "\[usize; \d+\]" -> "[usize; N]" +// FIXME https://github.com/rust-lang/rust/issues/59774 +// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> "" +// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> "" + #[cfg(target_pointer_width = "32")] fn main() { let x = [0usize; 0xffff_ffff]; diff --git a/src/test/ui/issues/issue-17913.rs b/src/test/ui/issues/issue-17913.rs index b0c4ef54b165d..48d8f407aa1e4 100644 --- a/src/test/ui/issues/issue-17913.rs +++ b/src/test/ui/issues/issue-17913.rs @@ -1,6 +1,10 @@ // normalize-stderr-test "\[&usize; \d+\]" -> "[&usize; N]" // error-pattern: too big for the current architecture +// FIXME https://github.com/rust-lang/rust/issues/59774 +// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> "" +// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> "" + #![feature(box_syntax)] #[cfg(target_pointer_width = "64")] diff --git a/src/test/ui/linkage2.rs b/src/test/ui/linkage2.rs index 6d1410a90bd21..fa4221fb3392e 100644 --- a/src/test/ui/linkage2.rs +++ b/src/test/ui/linkage2.rs @@ -1,3 +1,7 @@ +// FIXME https://github.com/rust-lang/rust/issues/59774 +// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> "" +// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> "" + #![feature(linkage)] extern { diff --git a/src/test/ui/linkage2.stderr b/src/test/ui/linkage2.stderr index 64213f1270adb..c72978388ebfe 100644 --- a/src/test/ui/linkage2.stderr +++ b/src/test/ui/linkage2.stderr @@ -1,5 +1,5 @@ error: must have type `*const T` or `*mut T` - --> $DIR/linkage2.rs:4:32 + --> $DIR/linkage2.rs:8:32 | LL | #[linkage = "extern_weak"] static foo: i32; | ^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/linkage3.rs b/src/test/ui/linkage3.rs index f094a0d53e941..1462079acf7e7 100644 --- a/src/test/ui/linkage3.rs +++ b/src/test/ui/linkage3.rs @@ -1,3 +1,7 @@ +// FIXME https://github.com/rust-lang/rust/issues/59774 +// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> "" +// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> "" + #![feature(linkage)] extern { diff --git a/src/test/ui/linkage3.stderr b/src/test/ui/linkage3.stderr index a03593ff2c68e..b74fdc91429e2 100644 --- a/src/test/ui/linkage3.stderr +++ b/src/test/ui/linkage3.stderr @@ -1,5 +1,5 @@ error: invalid linkage specified - --> $DIR/linkage3.rs:4:24 + --> $DIR/linkage3.rs:8:24 | LL | #[linkage = "foo"] static foo: *const i32; | ^^^^^^^^^^^^^^^^^^^^^^^ From 8861232b5a068da7d5ed6553fe65783f39987570 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 9 Apr 2019 22:36:56 +0200 Subject: [PATCH 08/23] some more tests need normalization --- src/test/ui/huge-array-simple.rs | 4 ++++ src/test/ui/huge-struct.rs | 4 ++++ src/test/ui/issues/issue-56762.rs | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/src/test/ui/huge-array-simple.rs b/src/test/ui/huge-array-simple.rs index 8b244a47232fa..0ff27168a7d86 100644 --- a/src/test/ui/huge-array-simple.rs +++ b/src/test/ui/huge-array-simple.rs @@ -1,6 +1,10 @@ // error-pattern: too big for the current architecture // normalize-stderr-test "; \d+]" -> "; N]" + +// FIXME https://github.com/rust-lang/rust/issues/59774 +// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> "" +// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> "" #![allow(exceeding_bitshifts)] #[cfg(target_pointer_width = "64")] diff --git a/src/test/ui/huge-struct.rs b/src/test/ui/huge-struct.rs index 74e43cc6472be..dc7d75a6f028e 100644 --- a/src/test/ui/huge-struct.rs +++ b/src/test/ui/huge-struct.rs @@ -2,6 +2,10 @@ // normalize-stderr-test "S1M" -> "SXX" // error-pattern: too big for the current +// FIXME https://github.com/rust-lang/rust/issues/59774 +// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> "" +// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> "" + struct S32 { v0: T, v1: T, diff --git a/src/test/ui/issues/issue-56762.rs b/src/test/ui/issues/issue-56762.rs index 97b66b2c7c923..8bb81b907c9a4 100644 --- a/src/test/ui/issues/issue-56762.rs +++ b/src/test/ui/issues/issue-56762.rs @@ -1,4 +1,8 @@ // only-x86_64 + +// FIXME https://github.com/rust-lang/rust/issues/59774 +// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> "" +// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> "" const HUGE_SIZE: usize = !0usize / 8; From c440c0a0fd3c875defe5a832a212f305b355334c Mon Sep 17 00:00:00 2001 From: Albin Stjerna Date: Tue, 2 Apr 2019 15:54:31 +0200 Subject: [PATCH 09/23] update polonius-engine --- Cargo.lock | 8 ++++---- src/librustc/Cargo.toml | 2 +- src/librustc_mir/Cargo.toml | 2 +- src/librustc_mir/borrow_check/nll/mod.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c071a2e11d269..2bf8d81f1aa97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1914,7 +1914,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "polonius-engine" -version = "0.6.2" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "datafrog 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2368,7 +2368,7 @@ dependencies = [ "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", - "polonius-engine 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", + "polonius-engine 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-rayon 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-rayon-core 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_apfloat 0.0.0", @@ -2868,7 +2868,7 @@ dependencies = [ "graphviz 0.0.0", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "log_settings 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "polonius-engine 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", + "polonius-engine 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc 0.0.0", "rustc_apfloat 0.0.0", "rustc_data_structures 0.0.0", @@ -4190,7 +4190,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum phf_generator 0.7.22 (registry+https://github.com/rust-lang/crates.io-index)" = "05a079dd052e7b674d21cb31cbb6c05efd56a2cd2827db7692e2f1a507ebd998" "checksum phf_shared 0.7.22 (registry+https://github.com/rust-lang/crates.io-index)" = "c2261d544c2bb6aa3b10022b0be371b9c7c64f762ef28c6f5d4f1ef6d97b5930" "checksum pkg-config 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)" = "676e8eb2b1b4c9043511a9b7bea0915320d7e502b0a079fb03f9635a5252b18c" -"checksum polonius-engine 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "2490c396085801abf88df91758bad806b0890354f0875d624e62ecf0579a8145" +"checksum polonius-engine 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8b24942fee141ea45628484a453762bb7e515099c3ec05fbeb76b7bf57b1aeed" "checksum precomputed-hash 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "925383efa346730478fb4838dbe9137d2a47675ad789c546d150a6e1dd4ab31c" "checksum pretty_assertions 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "3a029430f0d744bc3d15dd474d591bed2402b645d024583082b9f63bb936dac6" "checksum pretty_env_logger 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "df8b3f4e0475def7d9c2e5de8e5a1306949849761e107b360d03e98eafaffd61" diff --git a/src/librustc/Cargo.toml b/src/librustc/Cargo.toml index 31e10c19c7a60..f07111ef647da 100644 --- a/src/librustc/Cargo.toml +++ b/src/librustc/Cargo.toml @@ -19,7 +19,7 @@ lazy_static = "1.0.0" num_cpus = "1.0" scoped-tls = "1.0" log = { version = "0.4", features = ["release_max_level_info", "std"] } -polonius-engine = "0.6.2" +polonius-engine = "0.7.0" rustc-rayon = "0.1.2" rustc-rayon-core = "0.1.2" rustc_apfloat = { path = "../librustc_apfloat" } diff --git a/src/librustc_mir/Cargo.toml b/src/librustc_mir/Cargo.toml index c32bafa99205f..5de5f5e757119 100644 --- a/src/librustc_mir/Cargo.toml +++ b/src/librustc_mir/Cargo.toml @@ -15,7 +15,7 @@ either = "1.5.0" dot = { path = "../libgraphviz", package = "graphviz" } log = "0.4" log_settings = "0.1.1" -polonius-engine = "0.6.2" +polonius-engine = "0.7.0" rustc = { path = "../librustc" } rustc_target = { path = "../librustc_target" } rustc_data_structures = { path = "../librustc_data_structures" } diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index 2d3800dd1dda8..3e1b93fb4170a 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -173,7 +173,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>( if infcx.tcx.sess.opts.debugging_opts.polonius { let algorithm = env::var("POLONIUS_ALGORITHM") - .unwrap_or_else(|_| String::from("DatafrogOpt")); + .unwrap_or_else(|_| String::from("Hybrid")); let algorithm = Algorithm::from_str(&algorithm).unwrap(); debug!("compute_regions: using polonius algorithm {:?}", algorithm); Some(Rc::new(Output::compute( From 7b1df42accbfdf54148072c7a3d1e80177c388b0 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 10 Apr 2019 13:46:37 +0200 Subject: [PATCH 10/23] Clean up handling of -Zpgo-gen commandline option. --- src/librustc/session/config.rs | 38 ++++++++++++++++--- src/librustc_codegen_llvm/attributes.rs | 2 +- src/librustc_codegen_llvm/back/link.rs | 2 +- src/librustc_codegen_llvm/back/write.rs | 22 ++++++++--- .../back/symbol_export.rs | 2 +- src/librustc_codegen_ssa/back/write.rs | 7 ++-- src/librustc_metadata/creader.rs | 2 +- .../run-make-fulldeps/pgo-gen-lto/Makefile | 6 +-- src/test/run-make-fulldeps/pgo-gen/Makefile | 6 +-- 9 files changed, 60 insertions(+), 27 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 988e4a9ff23a2..7c0eab26b09b6 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -113,6 +113,21 @@ impl LinkerPluginLto { } } +#[derive(Clone, PartialEq, Hash)] +pub enum PgoGenerate { + Enabled(Option), + Disabled, +} + +impl PgoGenerate { + pub fn enabled(&self) -> bool { + match *self { + PgoGenerate::Enabled(_) => true, + PgoGenerate::Disabled => false, + } + } +} + #[derive(Clone, Copy, PartialEq, Hash)] pub enum DebugInfo { None, @@ -826,13 +841,15 @@ macro_rules! options { pub const parse_linker_plugin_lto: Option<&str> = Some("either a boolean (`yes`, `no`, `on`, `off`, etc), \ or the path to the linker plugin"); + pub const parse_pgo_generate: Option<&str> = + Some("an optional path to the profiling data output directory"); pub const parse_merge_functions: Option<&str> = Some("one of: `disabled`, `trampolines`, or `aliases`"); } #[allow(dead_code)] mod $mod_set { - use super::{$struct_name, Passes, Sanitizer, LtoCli, LinkerPluginLto}; + use super::{$struct_name, Passes, Sanitizer, LtoCli, LinkerPluginLto, PgoGenerate}; use rustc_target::spec::{LinkerFlavor, MergeFunctions, PanicStrategy, RelroLevel}; use std::path::PathBuf; use std::str::FromStr; @@ -1087,6 +1104,14 @@ macro_rules! options { true } + fn parse_pgo_generate(slot: &mut PgoGenerate, v: Option<&str>) -> bool { + *slot = match v { + None => PgoGenerate::Enabled(None), + Some(path) => PgoGenerate::Enabled(Some(PathBuf::from(path))), + }; + true + } + fn parse_merge_functions(slot: &mut Option, v: Option<&str>) -> bool { match v.and_then(|s| MergeFunctions::from_str(s).ok()) { Some(mergefunc) => *slot = Some(mergefunc), @@ -1363,7 +1388,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "extra arguments to prepend to the linker invocation (space separated)"), profile: bool = (false, parse_bool, [TRACKED], "insert profiling code"), - pgo_gen: Option = (None, parse_opt_string, [TRACKED], + pgo_gen: PgoGenerate = (PgoGenerate::Disabled, parse_pgo_generate, [TRACKED], "Generate PGO profile data, to a given file, or to the default location if it's empty."), pgo_use: String = (String::new(), parse_string, [TRACKED], "Use PGO profile data from the given profile file."), @@ -1980,7 +2005,7 @@ pub fn build_session_options_and_crate_config( ); } - if debugging_opts.pgo_gen.is_some() && !debugging_opts.pgo_use.is_empty() { + if debugging_opts.pgo_gen.enabled() && !debugging_opts.pgo_use.is_empty() { early_error( error_format, "options `-Z pgo-gen` and `-Z pgo-use` are exclusive", @@ -2490,7 +2515,7 @@ mod dep_tracking { use std::path::PathBuf; use std::collections::hash_map::DefaultHasher; use super::{CrateType, DebugInfo, ErrorOutputType, OptLevel, OutputTypes, - Passes, Sanitizer, LtoCli, LinkerPluginLto}; + Passes, Sanitizer, LtoCli, LinkerPluginLto, PgoGenerate}; use syntax::feature_gate::UnstableFeatures; use rustc_target::spec::{MergeFunctions, PanicStrategy, RelroLevel, TargetTriple}; use syntax::edition::Edition; @@ -2558,6 +2583,7 @@ mod dep_tracking { impl_dep_tracking_hash_via_hash!(TargetTriple); impl_dep_tracking_hash_via_hash!(Edition); impl_dep_tracking_hash_via_hash!(LinkerPluginLto); + impl_dep_tracking_hash_via_hash!(PgoGenerate); impl_dep_tracking_hash_for_sortable_vec_of!(String); impl_dep_tracking_hash_for_sortable_vec_of!(PathBuf); @@ -2625,7 +2651,7 @@ mod tests { build_session_options_and_crate_config, to_crate_config }; - use crate::session::config::{LtoCli, LinkerPluginLto}; + use crate::session::config::{LtoCli, LinkerPluginLto, PgoGenerate}; use crate::session::build_session; use crate::session::search_paths::SearchPath; use std::collections::{BTreeMap, BTreeSet}; @@ -3124,7 +3150,7 @@ mod tests { assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash()); opts = reference.clone(); - opts.debugging_opts.pgo_gen = Some(String::from("abc")); + opts.debugging_opts.pgo_gen = PgoGenerate::Enabled(None); assert_ne!(reference.dep_tracking_hash(), opts.dep_tracking_hash()); opts = reference.clone(); diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index 77fa34e74dd70..b15a64c966b1b 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -104,7 +104,7 @@ pub fn set_probestack(cx: &CodegenCx<'ll, '_>, llfn: &'ll Value) { } // probestack doesn't play nice either with pgo-gen. - if cx.sess().opts.debugging_opts.pgo_gen.is_some() { + if cx.sess().opts.debugging_opts.pgo_gen.enabled() { return; } diff --git a/src/librustc_codegen_llvm/back/link.rs b/src/librustc_codegen_llvm/back/link.rs index 19419a72b94dd..6a3c2adc85618 100644 --- a/src/librustc_codegen_llvm/back/link.rs +++ b/src/librustc_codegen_llvm/back/link.rs @@ -1014,7 +1014,7 @@ fn link_args(cmd: &mut dyn Linker, cmd.build_static_executable(); } - if sess.opts.debugging_opts.pgo_gen.is_some() { + if sess.opts.debugging_opts.pgo_gen.enabled() { cmd.pgo_gen(); } diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index f0ed201ad5c27..4de65325637ff 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -13,7 +13,7 @@ use crate::LlvmCodegenBackend; use rustc::hir::def_id::LOCAL_CRATE; use rustc_codegen_ssa::back::write::{CodegenContext, ModuleConfig, run_assembler}; use rustc_codegen_ssa::traits::*; -use rustc::session::config::{self, OutputType, Passes, Lto}; +use rustc::session::config::{self, OutputType, Passes, Lto, PgoGenerate}; use rustc::session::Session; use rustc::ty::TyCtxt; use rustc_codegen_ssa::{ModuleCodegen, CompiledModule}; @@ -26,7 +26,7 @@ use errors::{Handler, FatalError}; use std::ffi::{CString, CStr}; use std::fs; use std::io::{self, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::str; use std::sync::Arc; use std::slice; @@ -708,10 +708,20 @@ pub unsafe fn with_llvm_pmb(llmod: &llvm::Module, .unwrap_or(llvm::CodeGenOptSizeNone); let inline_threshold = config.inline_threshold; - let pgo_gen_path = config.pgo_gen.as_ref().map(|s| { - let s = if s.is_empty() { "default_%m.profraw" } else { s }; - CString::new(s.as_bytes()).unwrap() - }); + let pgo_gen_path = match config.pgo_gen { + PgoGenerate::Enabled(ref opt_dir_path) => { + let path = if let Some(dir_path) = opt_dir_path { + dir_path.join("default_%m.profraw") + } else { + PathBuf::from("default_%m.profraw") + }; + + Some(CString::new(format!("{}", path.display())).unwrap()) + } + PgoGenerate::Disabled => { + None + } + }; let pgo_use_path = if config.pgo_use.is_empty() { None diff --git a/src/librustc_codegen_ssa/back/symbol_export.rs b/src/librustc_codegen_ssa/back/symbol_export.rs index 336f41b784a81..a55f783df43a3 100644 --- a/src/librustc_codegen_ssa/back/symbol_export.rs +++ b/src/librustc_codegen_ssa/back/symbol_export.rs @@ -209,7 +209,7 @@ fn exported_symbols_provider_local<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } - if tcx.sess.opts.debugging_opts.pgo_gen.is_some() { + if tcx.sess.opts.debugging_opts.pgo_gen.enabled() { // These are weak symbols that point to the profile version and the // profile name, which need to be treated as exported so LTO doesn't nix // them. diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index fa8c4177eafe2..7fc43f29701ad 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -12,7 +12,8 @@ use rustc_incremental::{copy_cgu_workproducts_to_incr_comp_cache_dir, use rustc::dep_graph::{WorkProduct, WorkProductId, WorkProductFileKind}; use rustc::dep_graph::cgu_reuse_tracker::CguReuseTracker; use rustc::middle::cstore::EncodedMetadata; -use rustc::session::config::{self, OutputFilenames, OutputType, Passes, Sanitizer, Lto}; +use rustc::session::config::{self, OutputFilenames, OutputType, Passes, Lto, + Sanitizer, PgoGenerate}; use rustc::session::Session; use rustc::util::nodemap::FxHashMap; use rustc::hir::def_id::{CrateNum, LOCAL_CRATE}; @@ -56,7 +57,7 @@ pub struct ModuleConfig { /// Some(level) to optimize binary size, or None to not affect program size. pub opt_size: Option, - pub pgo_gen: Option, + pub pgo_gen: PgoGenerate, pub pgo_use: String, // Flags indicating which outputs to produce. @@ -94,7 +95,7 @@ impl ModuleConfig { opt_level: None, opt_size: None, - pgo_gen: None, + pgo_gen: PgoGenerate::Disabled, pgo_use: String::new(), emit_no_opt_bc: false, diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 36d9bf9f50dd5..66daa4518bef6 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -862,7 +862,7 @@ impl<'a> CrateLoader<'a> { fn inject_profiler_runtime(&mut self) { if self.sess.opts.debugging_opts.profile || - self.sess.opts.debugging_opts.pgo_gen.is_some() + self.sess.opts.debugging_opts.pgo_gen.enabled() { info!("loading profiler"); diff --git a/src/test/run-make-fulldeps/pgo-gen-lto/Makefile b/src/test/run-make-fulldeps/pgo-gen-lto/Makefile index e358314c0d09e..7c19961b1e420 100644 --- a/src/test/run-make-fulldeps/pgo-gen-lto/Makefile +++ b/src/test/run-make-fulldeps/pgo-gen-lto/Makefile @@ -1,10 +1,8 @@ -include ../tools.mk -# ignore-windows - all: ifeq ($(PROFILER_SUPPORT),1) - $(RUSTC) -Copt-level=3 -Clto=fat -Z pgo-gen="$(TMPDIR)/test.profraw" test.rs + $(RUSTC) -Copt-level=3 -Clto=fat -Z pgo-gen="$(TMPDIR)" test.rs $(call RUN,test) || exit 1 - [ -e "$(TMPDIR)/test.profraw" ] || (echo "No .profraw file"; exit 1) + [ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1) endif diff --git a/src/test/run-make-fulldeps/pgo-gen/Makefile b/src/test/run-make-fulldeps/pgo-gen/Makefile index 1961dff8d783e..0469c4443d85a 100644 --- a/src/test/run-make-fulldeps/pgo-gen/Makefile +++ b/src/test/run-make-fulldeps/pgo-gen/Makefile @@ -1,10 +1,8 @@ -include ../tools.mk -# ignore-windows - all: ifeq ($(PROFILER_SUPPORT),1) - $(RUSTC) -g -Z pgo-gen="$(TMPDIR)/test.profraw" test.rs + $(RUSTC) -g -Z pgo-gen="$(TMPDIR)" test.rs $(call RUN,test) || exit 1 - [ -e "$(TMPDIR)/test.profraw" ] || (echo "No .profraw file"; exit 1) + [ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1) endif From d84907bbccf0430470ba7b8f121bb4f924cdffa4 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 7 Apr 2019 19:56:41 +0200 Subject: [PATCH 11/23] Suggest macro import from crate root. This commit suggests importing a macro from the root of a crate as the intent may have been to import a macro from the definition location that was annotated with `#[macro_export]`. --- src/librustc_resolve/error_reporting.rs | 82 +++++++++++++++++++++---- src/librustc_resolve/lib.rs | 10 +++ src/librustc_resolve/resolve_imports.rs | 55 +++++++++-------- src/test/ui/issue-59764.fixed | 15 +++++ src/test/ui/issue-59764.rs | 1 + src/test/ui/issue-59764.stderr | 12 +++- 6 files changed, 134 insertions(+), 41 deletions(-) create mode 100644 src/test/ui/issue-59764.fixed diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index 461d02e515d38..99aa6e39f7b71 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -5,15 +5,16 @@ use log::debug; use rustc::hir::def::{Def, CtorKind, Namespace::*}; use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId}; use rustc::session::config::nightly_options; -use syntax::ast::{Expr, ExprKind}; +use syntax::ast::{Expr, ExprKind, Ident}; +use syntax::ext::base::MacroKind; use syntax::symbol::keywords; use syntax_pos::Span; use crate::macros::ParentScope; -use crate::resolve_imports::ImportResolver; +use crate::resolve_imports::{ImportDirective, ImportResolver}; use crate::{import_candidate_to_enum_paths, is_self_type, is_self_value, path_names_to_string}; use crate::{AssocSuggestion, CrateLint, ImportSuggestion, ModuleOrUniformRoot, PathResult, - PathSource, Resolver, Segment}; + PathSource, Resolver, Segment, Suggestion}; impl<'a> Resolver<'a> { /// Handles error reporting for `smart_resolve_path_fragment` function. @@ -428,7 +429,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { debug!("make_path_suggestion: span={:?} path={:?}", span, path); match (path.get(0), path.get(1)) { @@ -463,13 +464,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { // Replace first ident with `self` and check if that is valid. path[0].ident.name = keywords::SelfLower.name(); let result = self.resolve_path(&path, None, parent_scope, false, span, CrateLint::No); debug!("make_missing_self_suggestion: path={:?} result={:?}", path, result); if let PathResult::Module(..) = result { - Some((path, None)) + Some((path, Vec::new())) } else { None } @@ -487,7 +488,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { // Replace first ident with `crate` and check if that is valid. path[0].ident.name = keywords::Crate.name(); let result = self.resolve_path(&path, None, parent_scope, false, span, CrateLint::No); @@ -495,11 +496,11 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { if let PathResult::Module(..) = result { Some(( path, - Some( + vec![ "`use` statements changed in Rust 2018; read more at \ ".to_string() - ), + ], )) } else { None @@ -518,13 +519,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { // Replace first ident with `crate` and check if that is valid. path[0].ident.name = keywords::Super.name(); let result = self.resolve_path(&path, None, parent_scope, false, span, CrateLint::No); debug!("make_missing_super_suggestion: path={:?} result={:?}", path, result); if let PathResult::Module(..) = result { - Some((path, None)) + Some((path, Vec::new())) } else { None } @@ -545,7 +546,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { if path[1].ident.span.rust_2015() { return None; } @@ -564,10 +565,65 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { debug!("make_external_crate_suggestion: name={:?} path={:?} result={:?}", name, path, result); if let PathResult::Module(..) = result { - return Some((path, None)); + return Some((path, Vec::new())); } } None } + + /// Suggests importing a macro from the root of the crate rather than a module within + /// the crate. + /// + /// ``` + /// help: a macro with this name exists at the root of the crate + /// | + /// LL | use issue_59764::makro; + /// | ^^^^^^^^^^^^^^^^^^ + /// | + /// = note: this could be because a macro annotated with `#[macro_export]` will be exported + /// at the root of the crate instead of the module where it is defined + /// ``` + pub(crate) fn check_for_module_export_macro( + &self, + directive: &'b ImportDirective<'b>, + module: ModuleOrUniformRoot<'b>, + ident: Ident, + ) -> Option<(Option, Vec)> { + let mut crate_module = if let ModuleOrUniformRoot::Module(module) = module { + module + } else { + return None; + }; + + while let Some(parent) = crate_module.parent { + crate_module = parent; + } + + if ModuleOrUniformRoot::same_def(ModuleOrUniformRoot::Module(crate_module), module) { + // Don't make a suggestion if the import was already from the root of the + // crate. + return None; + } + + let resolutions = crate_module.resolutions.borrow(); + let resolution = resolutions.get(&(ident, MacroNS))?; + let binding = resolution.borrow().binding()?; + if let Def::Macro(_, MacroKind::Bang) = binding.def() { + let name = crate_module.kind.name().unwrap(); + let suggestion = Some(( + directive.span, + String::from("a macro with this name exists at the root of the crate"), + format!("{}::{}", name, ident), + Applicability::MaybeIncorrect, + )); + let note = vec![ + "this could be because a macro annotated with `#[macro_export]` will be exported \ + at the root of the crate instead of the module where it is defined".to_string(), + ]; + Some((suggestion, note)) + } else { + None + } + } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 5216156c0cab8..0fb1ca4298358 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1091,6 +1091,16 @@ enum ModuleKind { Def(Def, Name), } +impl ModuleKind { + /// Get name of the module. + pub fn name(&self) -> Option { + match self { + ModuleKind::Block(..) => None, + ModuleKind::Def(_, name) => Some(*name), + } + } +} + /// One node in the tree of modules. pub struct ModuleData<'a> { parent: Option>, diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 97b630ba5f298..6a01af5d1153a 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -145,7 +145,7 @@ pub struct NameResolution<'a> { impl<'a> NameResolution<'a> { // Returns the binding for the name if it is known or None if it not known. - fn binding(&self) -> Option<&'a NameBinding<'a>> { + pub(crate) fn binding(&self) -> Option<&'a NameBinding<'a>> { self.binding.and_then(|binding| { if !binding.is_glob_import() || self.single_imports.is_empty() { Some(binding) } else { None } @@ -636,7 +636,7 @@ impl<'a> Resolver<'a> { struct UnresolvedImportError { span: Span, label: Option, - note: Option, + note: Vec, suggestion: Option, } @@ -756,8 +756,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { /// Upper limit on the number of `span_label` messages. const MAX_LABEL_COUNT: usize = 10; - let (span, msg, note) = if errors.is_empty() { - (span.unwrap(), "unresolved import".to_string(), None) + let (span, msg) = if errors.is_empty() { + (span.unwrap(), "unresolved import".to_string()) } else { let span = MultiSpan::from_spans( errors @@ -766,11 +766,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { .collect(), ); - let note = errors - .iter() - .filter_map(|(_, err)| err.note.as_ref()) - .last(); - let paths = errors .iter() .map(|(path, _)| format!("`{}`", path)) @@ -782,13 +777,15 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { paths.join(", "), ); - (span, msg, note) + (span, msg) }; let mut diag = struct_span_err!(self.resolver.session, span, E0432, "{}", &msg); - if let Some(note) = ¬e { - diag.note(note); + if let Some((_, UnresolvedImportError { note, .. })) = errors.iter().last() { + for message in note { + diag.note(&message); + } } for (_, err) in errors.into_iter().take(MAX_LABEL_COUNT) { @@ -961,7 +958,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { UnresolvedImportError { span, label: Some(label), - note: None, + note: Vec::new(), suggestion, } } @@ -1006,7 +1003,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { return Some(UnresolvedImportError { span: directive.span, label: Some(String::from("cannot glob-import a module into itself")), - note: None, + note: Vec::new(), suggestion: None, }); } @@ -1114,15 +1111,22 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } }); - let lev_suggestion = - find_best_match_for_name(names, &ident.as_str(), None).map(|suggestion| { - ( - ident.span, - String::from("a similar name exists in the module"), - suggestion.to_string(), - Applicability::MaybeIncorrect, - ) - }); + let (suggestion, note) = if let Some((suggestion, note)) = + self.check_for_module_export_macro(directive, module, ident) + { + + ( + suggestion.or_else(|| + find_best_match_for_name(names, &ident.as_str(), None) + .map(|suggestion| + (ident.span, String::from("a similar name exists in the module"), + suggestion.to_string(), Applicability::MaybeIncorrect) + )), + note, + ) + } else { + (None, Vec::new()) + }; let label = match module { ModuleOrUniformRoot::Module(module) => { @@ -1143,11 +1147,12 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } } }; + Some(UnresolvedImportError { span: directive.span, label: Some(label), - note: None, - suggestion: lev_suggestion, + note, + suggestion, }) } else { // `resolve_ident_in_module` reported a privacy error. diff --git a/src/test/ui/issue-59764.fixed b/src/test/ui/issue-59764.fixed new file mode 100644 index 0000000000000..d7a01ce5f4d74 --- /dev/null +++ b/src/test/ui/issue-59764.fixed @@ -0,0 +1,15 @@ +// aux-build:issue-59764.rs +// compile-flags:--extern issue_59764 +// edition:2018 +// run-rustfix + +use issue_59764::makro; +//~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] + +makro!(bar); +//~^ ERROR cannot determine resolution for the macro `makro` + +fn main() { + bar(); + //~^ ERROR cannot find function `bar` in this scope [E0425] +} diff --git a/src/test/ui/issue-59764.rs b/src/test/ui/issue-59764.rs index 8ac9b9eaba181..e6f7205bfca33 100644 --- a/src/test/ui/issue-59764.rs +++ b/src/test/ui/issue-59764.rs @@ -1,6 +1,7 @@ // aux-build:issue-59764.rs // compile-flags:--extern issue_59764 // edition:2018 +// run-rustfix use issue_59764::foo::makro; //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] diff --git a/src/test/ui/issue-59764.stderr b/src/test/ui/issue-59764.stderr index a428488b009ec..6b3237e9272c5 100644 --- a/src/test/ui/issue-59764.stderr +++ b/src/test/ui/issue-59764.stderr @@ -1,11 +1,17 @@ error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:5:5 + --> $DIR/issue-59764.rs:6:5 | LL | use issue_59764::foo::makro; | ^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::makro; + | ^^^^^^^^^^^^^^^^^^ error: cannot determine resolution for the macro `makro` - --> $DIR/issue-59764.rs:8:1 + --> $DIR/issue-59764.rs:9:1 | LL | makro!(bar); | ^^^^^ @@ -13,7 +19,7 @@ LL | makro!(bar); = note: import resolution is stuck, try simplifying macro imports error[E0425]: cannot find function `bar` in this scope - --> $DIR/issue-59764.rs:12:5 + --> $DIR/issue-59764.rs:13:5 | LL | bar(); | ^^^ not found in this scope From d589cf911109149564c8898ad1cb1d906f91caea Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 7 Apr 2019 20:34:21 +0200 Subject: [PATCH 12/23] Handle renamed imports. This commit extends the suggestion to handle imports that are aliased to another name. --- src/librustc_resolve/error_reporting.rs | 11 ++++++++--- src/librustc_resolve/resolve_imports.rs | 26 +++++++++++-------------- src/test/ui/issue-59764.fixed | 15 ++++++++++++++ src/test/ui/issue-59764.rs | 15 ++++++++++++++ src/test/ui/issue-59764.stderr | 20 +++++++++++++++---- 5 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index 99aa6e39f7b71..b7ccbbf607913 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -11,7 +11,7 @@ use syntax::symbol::keywords; use syntax_pos::Span; use crate::macros::ParentScope; -use crate::resolve_imports::{ImportDirective, ImportResolver}; +use crate::resolve_imports::{ImportDirective, ImportDirectiveSubclass, ImportResolver}; use crate::{import_candidate_to_enum_paths, is_self_type, is_self_value, path_names_to_string}; use crate::{AssocSuggestion, CrateLint, ImportSuggestion, ModuleOrUniformRoot, PathResult, PathSource, Resolver, Segment, Suggestion}; @@ -610,11 +610,16 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let resolution = resolutions.get(&(ident, MacroNS))?; let binding = resolution.borrow().binding()?; if let Def::Macro(_, MacroKind::Bang) = binding.def() { - let name = crate_module.kind.name().unwrap(); + let module_name = crate_module.kind.name().unwrap(); + let import = match directive.subclass { + ImportDirectiveSubclass::SingleImport { source, target, .. } if source != target => + format!("{} as {}", source, target), + _ => format!("{}", ident), + }; let suggestion = Some(( directive.span, String::from("a macro with this name exists at the root of the crate"), - format!("{}::{}", name, ident), + format!("{}::{}", module_name, import), Applicability::MaybeIncorrect, )); let note = vec![ diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 6a01af5d1153a..ef5f0f54ee369 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -1111,21 +1111,17 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } }); - let (suggestion, note) = if let Some((suggestion, note)) = - self.check_for_module_export_macro(directive, module, ident) - { - - ( - suggestion.or_else(|| - find_best_match_for_name(names, &ident.as_str(), None) - .map(|suggestion| - (ident.span, String::from("a similar name exists in the module"), - suggestion.to_string(), Applicability::MaybeIncorrect) - )), - note, - ) - } else { - (None, Vec::new()) + let lev_suggestion = find_best_match_for_name(names, &ident.as_str(), None) + .map(|suggestion| + (ident.span, String::from("a similar name exists in the module"), + suggestion.to_string(), Applicability::MaybeIncorrect) + ); + + let (suggestion, note) = match self.check_for_module_export_macro( + directive, module, ident, + ) { + Some((suggestion, note)) => (suggestion.or(lev_suggestion), note), + _ => (lev_suggestion, Vec::new()), }; let label = match module { diff --git a/src/test/ui/issue-59764.fixed b/src/test/ui/issue-59764.fixed index d7a01ce5f4d74..34b9fc4edd9b7 100644 --- a/src/test/ui/issue-59764.fixed +++ b/src/test/ui/issue-59764.fixed @@ -3,6 +3,21 @@ // edition:2018 // run-rustfix +#![allow(warnings)] + +// This tests the suggestion to import macros from the root of a crate. This aims to capture +// the case where a user attempts to import a macro from the definition location instead of the +// root of the crate and the macro is annotated with `#![macro_export]`. + +// Edge cases.. + +mod renamed_import { + use issue_59764::makro as baz; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +// Simple case.. + use issue_59764::makro; //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] diff --git a/src/test/ui/issue-59764.rs b/src/test/ui/issue-59764.rs index e6f7205bfca33..b33b8e0cf5c57 100644 --- a/src/test/ui/issue-59764.rs +++ b/src/test/ui/issue-59764.rs @@ -3,6 +3,21 @@ // edition:2018 // run-rustfix +#![allow(warnings)] + +// This tests the suggestion to import macros from the root of a crate. This aims to capture +// the case where a user attempts to import a macro from the definition location instead of the +// root of the crate and the macro is annotated with `#![macro_export]`. + +// Edge cases.. + +mod renamed_import { + use issue_59764::foo::makro as baz; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +// Simple case.. + use issue_59764::foo::makro; //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] diff --git a/src/test/ui/issue-59764.stderr b/src/test/ui/issue-59764.stderr index 6b3237e9272c5..8bb4d03fc5dd6 100644 --- a/src/test/ui/issue-59764.stderr +++ b/src/test/ui/issue-59764.stderr @@ -1,5 +1,17 @@ error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:6:5 + --> $DIR/issue-59764.rs:15:9 + | +LL | use issue_59764::foo::makro as baz; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::makro as baz; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:21:5 | LL | use issue_59764::foo::makro; | ^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo` @@ -11,7 +23,7 @@ LL | use issue_59764::makro; | ^^^^^^^^^^^^^^^^^^ error: cannot determine resolution for the macro `makro` - --> $DIR/issue-59764.rs:9:1 + --> $DIR/issue-59764.rs:24:1 | LL | makro!(bar); | ^^^^^ @@ -19,12 +31,12 @@ LL | makro!(bar); = note: import resolution is stuck, try simplifying macro imports error[E0425]: cannot find function `bar` in this scope - --> $DIR/issue-59764.rs:13:5 + --> $DIR/issue-59764.rs:28:5 | LL | bar(); | ^^^ not found in this scope -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors Some errors occurred: E0425, E0432. For more information about an error, try `rustc --explain E0425`. From 7c955409e3cb06b2a5c008e80c6856cb25a74b33 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 7 Apr 2019 23:18:13 +0200 Subject: [PATCH 13/23] Handle edge cases. This commit introduces more dirty span manipulation into the compiler in order to handle the various edge cases in moving/renaming the macro import so it is at the root of the import. --- src/librustc_resolve/error_reporting.rs | 230 +++++++++++++++++++++++- src/librustc_resolve/lib.rs | 77 ++------ src/test/ui/auxiliary/issue-59764.rs | 10 ++ src/test/ui/issue-59764.fixed | 93 ++++++++++ src/test/ui/issue-59764.rs | 93 ++++++++++ src/test/ui/issue-59764.stderr | 195 +++++++++++++++++++- 6 files changed, 625 insertions(+), 73 deletions(-) diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index b7ccbbf607913..eea8cb80f69ef 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -4,11 +4,11 @@ use errors::{Applicability, DiagnosticBuilder, DiagnosticId}; use log::debug; use rustc::hir::def::{Def, CtorKind, Namespace::*}; use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId}; -use rustc::session::config::nightly_options; +use rustc::session::{Session, config::nightly_options}; use syntax::ast::{Expr, ExprKind, Ident}; use syntax::ext::base::MacroKind; -use syntax::symbol::keywords; -use syntax_pos::Span; +use syntax::symbol::{Symbol, keywords}; +use syntax_pos::{BytePos, Span}; use crate::macros::ParentScope; use crate::resolve_imports::{ImportDirective, ImportDirectiveSubclass, ImportResolver}; @@ -616,10 +616,84 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { format!("{} as {}", source, target), _ => format!("{}", ident), }; + + // Assume this is the easy case of `use issue_59764::foo::makro;` and just remove + // intermediate segments. + let (mut span, mut correction) = (directive.span, + format!("{}::{}", module_name, import)); + + if directive.is_nested() { + span = directive.use_span; + + // Find the binding span (and any trailing commas and spaces). + // ie. `use a::b::{c, d, e};` + // ^^^ + let (found_closing_brace, binding_span) = find_span_of_binding_until_next_binding( + self.resolver.session, directive.span, directive.use_span, + ); + debug!("check_for_module_export_macro: found_closing_brace={:?} binding_span={:?}", + found_closing_brace, binding_span); + + let mut removal_span = binding_span; + if found_closing_brace { + // If the binding span ended with a closing brace, as in the below example: + // ie. `use a::b::{c, d};` + // ^ + // Then expand the span of characters to remove to include the previous + // binding's trailing comma. + // ie. `use a::b::{c, d};` + // ^^^ + if let Some(previous_span) = extend_span_to_previous_binding( + self.resolver.session, binding_span, + ) { + debug!("check_for_module_export_macro: previous_span={:?}", previous_span); + removal_span = removal_span.with_lo(previous_span.lo()); + } + } + debug!("check_for_module_export_macro: removal_span={:?}", removal_span); + + // Find the span after the crate name and if it has nested imports immediatately + // after the crate name already. + // ie. `use a::b::{c, d};` + // ^^^^^^^^^ + // or `use a::{b, c, d}};` + // ^^^^^^^^^^^ + let (has_nested, after_crate_name) = find_span_immediately_after_crate_name( + self.resolver.session, module_name, directive.use_span, + ); + debug!("check_for_module_export_macro: has_nested={:?} after_crate_name={:?}", + has_nested, after_crate_name); + + let source_map = self.resolver.session.source_map(); + + // Remove two bytes at the end to keep all but the `};` characters. + // ie. `{b::{c, d}, e::{f, g}};` + // ^^^^^^^^^^^^^^^^^^^^^ + let end_bytes = BytePos(if has_nested { 2 } else { 1 }); + let mut remaining_span = after_crate_name.with_hi( + after_crate_name.hi() - end_bytes); + if has_nested { + // Remove two bytes at the start to keep all but the initial `{` character. + // ie. `{b::{c, d}, e::{f, g}` + // ^^^^^^^^^^^^^^^^^^^^ + remaining_span = remaining_span.with_lo(after_crate_name.lo() + BytePos(1)); + } + + // Calculate the number of characters into a snippet to remove the removal + // span. + let lo = removal_span.lo() - remaining_span.lo(); + let hi = lo + (removal_span.hi() - removal_span.lo()); + if let Ok(mut remaining) = source_map.span_to_snippet(remaining_span) { + // Remove the original location of the binding. + remaining.replace_range((lo.0 as usize)..(hi.0 as usize), ""); + correction = format!("use {}::{{{}, {}}};", module_name, import, remaining); + } + } + let suggestion = Some(( - directive.span, + span, String::from("a macro with this name exists at the root of the crate"), - format!("{}::{}", module_name, import), + correction, Applicability::MaybeIncorrect, )); let note = vec![ @@ -632,3 +706,149 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } } } + +/// Given a `binding_span` of a binding within a use statement: +/// +/// ``` +/// use foo::{a, b, c}; +/// ^ +/// ``` +/// +/// then return the span until the next binding or the end of the statement: +/// +/// ``` +/// use foo::{a, b, c}; +/// ^^^ +/// ``` +pub(crate) fn find_span_of_binding_until_next_binding( + sess: &Session, + binding_span: Span, + use_span: Span, +) -> (bool, Span) { + let source_map = sess.source_map(); + + // Find the span of everything after the binding. + // ie. `a, e};` or `a};` + let binding_until_end = binding_span.with_hi(use_span.hi()); + + // Find everything after the binding but not including the binding. + // ie. `, e};` or `};` + let after_binding_until_end = binding_until_end.with_lo(binding_span.hi()); + + // Keep characters in the span until we encounter something that isn't a comma or + // whitespace. + // ie. `, ` or ``. + // + // Also note whether a closing brace character was encountered. If there + // was, then later go backwards to remove any trailing commas that are left. + let mut found_closing_brace = false; + let after_binding_until_next_binding = source_map.span_take_while( + after_binding_until_end, + |&ch| { + if ch == '}' { found_closing_brace = true; } + ch == ' ' || ch == ',' + } + ); + + // Combine the two spans. + // ie. `a, ` or `a`. + // + // Removing these would leave `issue_52891::{d, e};` or `issue_52891::{d, e, };` + let span = binding_span.with_hi(after_binding_until_next_binding.hi()); + + (found_closing_brace, span) +} + +/// Given a `binding_span`, return the span through to the comma or opening brace of the previous +/// binding. +/// +/// ``` +/// use foo::a::{a, b, c}; +/// ^^--- binding span +/// | +/// returned span +/// +/// use foo::{a, b, c}; +/// --- binding span +/// ``` +pub(crate) fn extend_span_to_previous_binding( + sess: &Session, + binding_span: Span, +) -> Option { + let source_map = sess.source_map(); + + // `prev_source` will contain all of the source that came before the span. + // Then split based on a command and take the first (ie. closest to our span) + // snippet. In the example, this is a space. + let prev_source = source_map.span_to_prev_source(binding_span).ok()?; + + let prev_comma = prev_source.rsplit(',').collect::>(); + let prev_starting_brace = prev_source.rsplit('{').collect::>(); + if prev_comma.len() <= 1 || prev_starting_brace.len() <= 1 { + return None; + } + + let prev_comma = prev_comma.first().unwrap(); + let prev_starting_brace = prev_starting_brace.first().unwrap(); + + // If the amount of source code before the comma is greater than + // the amount of source code before the starting brace then we've only + // got one item in the nested item (eg. `issue_52891::{self}`). + if prev_comma.len() > prev_starting_brace.len() { + return None; + } + + Some(binding_span.with_lo(BytePos( + // Take away the number of bytes for the characters we've found and an + // extra for the comma. + binding_span.lo().0 - (prev_comma.as_bytes().len() as u32) - 1 + ))) +} + +/// Given a `use_span` of a binding within a use statement, returns the highlighted span and if +/// it is a nested use tree. +/// +/// ``` +/// use foo::a::{b, c}; +/// ^^^^^^^^^^ // false +/// +/// use foo::{a, b, c}; +/// ^^^^^^^^^^ // true +/// +/// use foo::{a, b::{c, d}}; +/// ^^^^^^^^^^^^^^^ // true +/// ``` +fn find_span_immediately_after_crate_name( + sess: &Session, + module_name: Symbol, + use_span: Span, +) -> (bool, Span) { + debug!("find_span_immediately_after_crate_name: module_name={:?} use_span={:?}", + module_name, use_span); + let source_map = sess.source_map(); + + // Get position of the first `{` character for the use statement. + // ie. `use foo::{a, b::{c, d}};` + // ^ + let pos_of_use_tree_left_bracket = source_map.span_until_char(use_span, '{').hi(); + debug!("find_span_immediately_after_crate_name: pos_of_use_tree_left_bracket={:?}", + pos_of_use_tree_left_bracket); + + // Calculate the expected difference between the first `{` character and the start of a + // use statement. + // ie. `use foo::{..};` + // ^^^^ + // | ^^^ + // 4 | ^^ + // 3 | + // 2 + let expected_difference = BytePos((module_name.as_str().len() + 4 + 2) as u32); + debug!("find_span_immediately_after_crate_name: expected_difference={:?}", + expected_difference); + let actual_difference = pos_of_use_tree_left_bracket - use_span.lo(); + debug!("find_span_immediately_after_crate_name: actual_difference={:?}", + actual_difference); + + (expected_difference == actual_difference, + use_span.with_lo(use_span.lo() + expected_difference)) +} diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 0fb1ca4298358..b33356c2ba7ae 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -50,7 +50,7 @@ use syntax::ast::{QSelf, TraitItemKind, TraitRef, Ty, TyKind}; use syntax::ptr::P; use syntax::{span_err, struct_span_err, unwrap_or, walk_list}; -use syntax_pos::{BytePos, Span, DUMMY_SP, MultiSpan}; +use syntax_pos::{Span, DUMMY_SP, MultiSpan}; use errors::{Applicability, DiagnosticBuilder, DiagnosticId}; use log::debug; @@ -62,6 +62,7 @@ use std::mem::replace; use rustc_data_structures::ptr_key::PtrKey; use rustc_data_structures::sync::Lrc; +use error_reporting::{find_span_of_binding_until_next_binding, extend_span_to_previous_binding}; use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution, ImportResolver}; use macros::{InvocationData, LegacyBinding, ParentScope}; @@ -5144,7 +5145,6 @@ impl<'a> Resolver<'a> { ) { assert!(directive.is_nested()); let message = "remove unnecessary import"; - let source_map = self.session.source_map(); // Two examples will be used to illustrate the span manipulations we're doing: // @@ -5153,73 +5153,24 @@ impl<'a> Resolver<'a> { // - Given `use issue_52891::{d, e, a};` where `a` is a duplicate then `binding_span` is // `a` and `directive.use_span` is `issue_52891::{d, e, a};`. - // Find the span of everything after the binding. - // ie. `a, e};` or `a};` - let binding_until_end = binding_span.with_hi(directive.use_span.hi()); - - // Find everything after the binding but not including the binding. - // ie. `, e};` or `};` - let after_binding_until_end = binding_until_end.with_lo(binding_span.hi()); - - // Keep characters in the span until we encounter something that isn't a comma or - // whitespace. - // ie. `, ` or ``. - // - // Also note whether a closing brace character was encountered. If there - // was, then later go backwards to remove any trailing commas that are left. - let mut found_closing_brace = false; - let after_binding_until_next_binding = source_map.span_take_while( - after_binding_until_end, - |&ch| { - if ch == '}' { found_closing_brace = true; } - ch == ' ' || ch == ',' - } + let (found_closing_brace, span) = find_span_of_binding_until_next_binding( + self.session, binding_span, directive.use_span, ); - // Combine the two spans. - // ie. `a, ` or `a`. - // - // Removing these would leave `issue_52891::{d, e};` or `issue_52891::{d, e, };` - let span = binding_span.with_hi(after_binding_until_next_binding.hi()); - // If there was a closing brace then identify the span to remove any trailing commas from // previous imports. if found_closing_brace { - if let Ok(prev_source) = source_map.span_to_prev_source(span) { - // `prev_source` will contain all of the source that came before the span. - // Then split based on a command and take the first (ie. closest to our span) - // snippet. In the example, this is a space. - let prev_comma = prev_source.rsplit(',').collect::>(); - let prev_starting_brace = prev_source.rsplit('{').collect::>(); - if prev_comma.len() > 1 && prev_starting_brace.len() > 1 { - let prev_comma = prev_comma.first().unwrap(); - let prev_starting_brace = prev_starting_brace.first().unwrap(); - - // If the amount of source code before the comma is greater than - // the amount of source code before the starting brace then we've only - // got one item in the nested item (eg. `issue_52891::{self}`). - if prev_comma.len() > prev_starting_brace.len() { - // So just remove the entire line... - err.span_suggestion( - directive.use_span_with_attributes, - message, - String::new(), - Applicability::MaybeIncorrect, - ); - return; - } - - let span = span.with_lo(BytePos( - // Take away the number of bytes for the characters we've found and an - // extra for the comma. - span.lo().0 - (prev_comma.as_bytes().len() as u32) - 1 - )); - err.tool_only_span_suggestion( - span, message, String::new(), Applicability::MaybeIncorrect, - ); - return; - } + if let Some(span) = extend_span_to_previous_binding(self.session, span) { + err.tool_only_span_suggestion(span, message, String::new(), + Applicability::MaybeIncorrect); + } else { + // Remove the entire line if we cannot extend the span back, this indicates a + // `issue_52891::{self}` case. + err.span_suggestion(directive.use_span_with_attributes, message, String::new(), + Applicability::MaybeIncorrect); } + + return; } err.span_suggestion(span, message, String::new(), Applicability::MachineApplicable); diff --git a/src/test/ui/auxiliary/issue-59764.rs b/src/test/ui/auxiliary/issue-59764.rs index 0243ef5c6c74a..a92eed968d060 100644 --- a/src/test/ui/auxiliary/issue-59764.rs +++ b/src/test/ui/auxiliary/issue-59764.rs @@ -5,4 +5,14 @@ pub mod foo { fn $foo() { } } } + + pub fn baz() {} + + pub fn foobar() {} + + pub mod barbaz { + pub fn barfoo() {} + } } + +pub fn foobaz() {} diff --git a/src/test/ui/issue-59764.fixed b/src/test/ui/issue-59764.fixed index 34b9fc4edd9b7..ff193fce7eb1d 100644 --- a/src/test/ui/issue-59764.fixed +++ b/src/test/ui/issue-59764.fixed @@ -11,11 +11,104 @@ // Edge cases.. +mod multiple_imports_same_line_at_end { + use issue_59764::{makro, foo::{baz}}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod multiple_imports_multiline_at_end_trailing_comma { + use issue_59764::{makro, foo::{ + baz, + //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + }}; +} + +mod multiple_imports_multiline_at_end { + use issue_59764::{makro, foo::{ + baz, + //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + }}; +} + +mod multiple_imports_same_line_in_middle { + use issue_59764::{makro, foo::{baz, foobar}}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod multiple_imports_multiline_in_middle_trailing_comma { + use issue_59764::{makro, foo::{ + baz, + //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + foobar, + }}; +} + +mod multiple_imports_multiline_in_middle { + use issue_59764::{makro, foo::{ + baz, + //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + foobar + }}; +} + +mod nested_imports { + use issue_59764::{makro, foobaz}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod nested_multiple_imports { + use issue_59764::{makro, foobaz, foo::{baz}}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod nested_multiline_multiple_imports_trailing_comma { + use issue_59764::{makro, + foobaz, + foo::{ + baz, + //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + }, + }; +} + +mod nested_multiline_multiple_imports { + use issue_59764::{makro, + foobaz, + foo::{ + baz, + //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + } + }; +} + +mod doubly_nested_multiple_imports { + use issue_59764::{makro, foobaz, foo::{baz, barbaz::{barfoo}}}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod doubly_multiline_nested_multiple_imports { + use issue_59764::{makro, + foobaz, + foo::{ + baz, + //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + barbaz::{ + barfoo, + } + } + }; +} + mod renamed_import { use issue_59764::makro as baz; //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] } +mod renamed_multiple_imports { + use issue_59764::{makro as foobar, foo::{baz}}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + // Simple case.. use issue_59764::makro; diff --git a/src/test/ui/issue-59764.rs b/src/test/ui/issue-59764.rs index b33b8e0cf5c57..c68471adaad56 100644 --- a/src/test/ui/issue-59764.rs +++ b/src/test/ui/issue-59764.rs @@ -11,11 +11,104 @@ // Edge cases.. +mod multiple_imports_same_line_at_end { + use issue_59764::foo::{baz, makro}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod multiple_imports_multiline_at_end_trailing_comma { + use issue_59764::foo::{ + baz, + makro, //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + }; +} + +mod multiple_imports_multiline_at_end { + use issue_59764::foo::{ + baz, + makro //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + }; +} + +mod multiple_imports_same_line_in_middle { + use issue_59764::foo::{baz, makro, foobar}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod multiple_imports_multiline_in_middle_trailing_comma { + use issue_59764::foo::{ + baz, + makro, //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + foobar, + }; +} + +mod multiple_imports_multiline_in_middle { + use issue_59764::foo::{ + baz, + makro, //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + foobar + }; +} + +mod nested_imports { + use issue_59764::{foobaz, foo::makro}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod nested_multiple_imports { + use issue_59764::{foobaz, foo::{baz, makro}}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod nested_multiline_multiple_imports_trailing_comma { + use issue_59764::{ + foobaz, + foo::{ + baz, + makro, //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + }, + }; +} + +mod nested_multiline_multiple_imports { + use issue_59764::{ + foobaz, + foo::{ + baz, + makro //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + } + }; +} + +mod doubly_nested_multiple_imports { + use issue_59764::{foobaz, foo::{baz, makro, barbaz::{barfoo}}}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + +mod doubly_multiline_nested_multiple_imports { + use issue_59764::{ + foobaz, + foo::{ + baz, + makro, //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + barbaz::{ + barfoo, + } + } + }; +} + mod renamed_import { use issue_59764::foo::makro as baz; //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] } +mod renamed_multiple_imports { + use issue_59764::foo::{baz, makro as foobar}; + //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] +} + // Simple case.. use issue_59764::foo::makro; diff --git a/src/test/ui/issue-59764.stderr b/src/test/ui/issue-59764.stderr index 8bb4d03fc5dd6..fe93547318468 100644 --- a/src/test/ui/issue-59764.stderr +++ b/src/test/ui/issue-59764.stderr @@ -1,5 +1,178 @@ error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:15:9 + --> $DIR/issue-59764.rs:15:33 + | +LL | use issue_59764::foo::{baz, makro}; + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, foo::{baz}}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:22:9 + | +LL | makro, + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, foo::{ +LL | baz, +LL | +LL | }}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:29:9 + | +LL | makro + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, foo::{ +LL | baz, +LL | +LL | }}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:34:33 + | +LL | use issue_59764::foo::{baz, makro, foobar}; + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, foo::{baz, foobar}}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:41:9 + | +LL | makro, + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, foo::{ +LL | baz, +LL | +LL | foobar, +LL | }}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:49:9 + | +LL | makro, + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, foo::{ +LL | baz, +LL | +LL | foobar +LL | }}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:55:31 + | +LL | use issue_59764::{foobaz, foo::makro}; + | ^^^^^^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, foobaz}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:60:42 + | +LL | use issue_59764::{foobaz, foo::{baz, makro}}; + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, foobaz, foo::{baz}}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:69:13 + | +LL | makro, + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, +LL | foobaz, +LL | foo::{ +LL | baz, +LL | +LL | }, + ... + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:79:13 + | +LL | makro + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, +LL | foobaz, +LL | foo::{ +LL | baz, +LL | +LL | } + ... + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:85:42 + | +LL | use issue_59764::{foobaz, foo::{baz, makro, barbaz::{barfoo}}}; + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, foobaz, foo::{baz, barbaz::{barfoo}}}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:94:13 + | +LL | makro, + | ^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro, +LL | foobaz, +LL | foo::{ +LL | baz, +LL | +LL | barbaz::{ + ... + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:103:9 | LL | use issue_59764::foo::makro as baz; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo` @@ -11,7 +184,19 @@ LL | use issue_59764::makro as baz; | ^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:21:5 + --> $DIR/issue-59764.rs:108:33 + | +LL | use issue_59764::foo::{baz, makro as foobar}; + | ^^^^^^^^^^^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro as foobar, foo::{baz}}; + | + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:114:5 | LL | use issue_59764::foo::makro; | ^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo` @@ -23,7 +208,7 @@ LL | use issue_59764::makro; | ^^^^^^^^^^^^^^^^^^ error: cannot determine resolution for the macro `makro` - --> $DIR/issue-59764.rs:24:1 + --> $DIR/issue-59764.rs:117:1 | LL | makro!(bar); | ^^^^^ @@ -31,12 +216,12 @@ LL | makro!(bar); = note: import resolution is stuck, try simplifying macro imports error[E0425]: cannot find function `bar` in this scope - --> $DIR/issue-59764.rs:28:5 + --> $DIR/issue-59764.rs:121:5 | LL | bar(); | ^^^ not found in this scope -error: aborting due to 4 previous errors +error: aborting due to 17 previous errors Some errors occurred: E0425, E0432. For more information about an error, try `rustc --explain E0425`. From 137ffa10229bfec497a446d24045388ee3c418d6 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 12 Apr 2019 01:19:02 +0200 Subject: [PATCH 14/23] Improve robustness of nested check. This commit removes the assumption that the start of a use statement will always be on one line with a single space - which was silly in the first place. --- src/librustc_resolve/error_reporting.rs | 49 +++++++++++++------------ src/test/ui/issue-59764.fixed | 11 ++++++ src/test/ui/issue-59764.rs | 14 +++++++ src/test/ui/issue-59764.stderr | 25 +++++++++++-- 4 files changed, 71 insertions(+), 28 deletions(-) diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index eea8cb80f69ef..5372c4ee44bdc 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -827,28 +827,29 @@ fn find_span_immediately_after_crate_name( module_name, use_span); let source_map = sess.source_map(); - // Get position of the first `{` character for the use statement. - // ie. `use foo::{a, b::{c, d}};` - // ^ - let pos_of_use_tree_left_bracket = source_map.span_until_char(use_span, '{').hi(); - debug!("find_span_immediately_after_crate_name: pos_of_use_tree_left_bracket={:?}", - pos_of_use_tree_left_bracket); - - // Calculate the expected difference between the first `{` character and the start of a - // use statement. - // ie. `use foo::{..};` - // ^^^^ - // | ^^^ - // 4 | ^^ - // 3 | - // 2 - let expected_difference = BytePos((module_name.as_str().len() + 4 + 2) as u32); - debug!("find_span_immediately_after_crate_name: expected_difference={:?}", - expected_difference); - let actual_difference = pos_of_use_tree_left_bracket - use_span.lo(); - debug!("find_span_immediately_after_crate_name: actual_difference={:?}", - actual_difference); - - (expected_difference == actual_difference, - use_span.with_lo(use_span.lo() + expected_difference)) + // Using `use issue_59764::foo::{baz, makro};` as an example throughout.. + let mut num_colons = 0; + // Find second colon.. `use issue_59764:` + let until_second_colon = source_map.span_take_while(use_span, |c| { + if *c == ':' { num_colons += 1; } + match c { + ':' if num_colons == 2 => false, + _ => true, + } + }); + // Find everything after the second colon.. `foo::{baz, makro};` + let from_second_colon = use_span.with_lo(until_second_colon.hi() + BytePos(1)); + + let mut found_a_non_whitespace_character = false; + // Find the first non-whitespace character in `from_second_colon`.. `f` + let after_second_colon = source_map.span_take_while(from_second_colon, |c| { + if found_a_non_whitespace_character { return false; } + if !c.is_whitespace() { found_a_non_whitespace_character = true; } + true + }); + + // Find the first `{` in from_second_colon.. `foo::{` + let next_left_bracket = source_map.span_through_char(from_second_colon, '{'); + + (next_left_bracket == after_second_colon, from_second_colon) } diff --git a/src/test/ui/issue-59764.fixed b/src/test/ui/issue-59764.fixed index ff193fce7eb1d..ad3bab50cfca3 100644 --- a/src/test/ui/issue-59764.fixed +++ b/src/test/ui/issue-59764.fixed @@ -109,6 +109,17 @@ mod renamed_multiple_imports { //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] } +mod lots_of_whitespace { + use issue_59764::{makro as foobar, + + foobaz, + + + foo::{baz} //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + + }; +} + // Simple case.. use issue_59764::makro; diff --git a/src/test/ui/issue-59764.rs b/src/test/ui/issue-59764.rs index c68471adaad56..0c6787691de0a 100644 --- a/src/test/ui/issue-59764.rs +++ b/src/test/ui/issue-59764.rs @@ -109,6 +109,20 @@ mod renamed_multiple_imports { //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] } +mod lots_of_whitespace { + use + issue_59764::{ + + foobaz, + + + foo::{baz, + + makro as foobar} //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] + + }; +} + // Simple case.. use issue_59764::foo::makro; diff --git a/src/test/ui/issue-59764.stderr b/src/test/ui/issue-59764.stderr index fe93547318468..89b1f84605dc7 100644 --- a/src/test/ui/issue-59764.stderr +++ b/src/test/ui/issue-59764.stderr @@ -196,7 +196,24 @@ LL | use issue_59764::{makro as foobar, foo::{baz}}; | error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:114:5 + --> $DIR/issue-59764.rs:121:17 + | +LL | makro as foobar} + | ^^^^^^^^^^^^^^^ no `makro` in `foo` + | + = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined +help: a macro with this name exists at the root of the crate + | +LL | use issue_59764::{makro as foobar, +LL | +LL | foobaz, +LL | +LL | +LL | foo::{baz} + ... + +error[E0432]: unresolved import `issue_59764::foo::makro` + --> $DIR/issue-59764.rs:128:5 | LL | use issue_59764::foo::makro; | ^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo` @@ -208,7 +225,7 @@ LL | use issue_59764::makro; | ^^^^^^^^^^^^^^^^^^ error: cannot determine resolution for the macro `makro` - --> $DIR/issue-59764.rs:117:1 + --> $DIR/issue-59764.rs:131:1 | LL | makro!(bar); | ^^^^^ @@ -216,12 +233,12 @@ LL | makro!(bar); = note: import resolution is stuck, try simplifying macro imports error[E0425]: cannot find function `bar` in this scope - --> $DIR/issue-59764.rs:121:5 + --> $DIR/issue-59764.rs:135:5 | LL | bar(); | ^^^ not found in this scope -error: aborting due to 17 previous errors +error: aborting due to 18 previous errors Some errors occurred: E0425, E0432. For more information about an error, try `rustc --explain E0425`. From 5158063c3ec1e1dc8d9b0a0806e29d6c6e54d765 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 12 Apr 2019 01:53:32 +0200 Subject: [PATCH 15/23] Switch to multipart suggestions. This commit changes the suggestion so that it is split into multiple parts in an effort to reduce the impact the applied suggestion could have on formatting. --- src/librustc_resolve/error_reporting.rs | 57 +++++----- src/librustc_resolve/lib.rs | 11 +- src/librustc_resolve/resolve_imports.rs | 12 +-- src/test/ui/issue-59764.fixed | 134 ------------------------ src/test/ui/issue-59764.rs | 1 - src/test/ui/issue-59764.stderr | 61 +++++------ 6 files changed, 68 insertions(+), 208 deletions(-) delete mode 100644 src/test/ui/issue-59764.fixed diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index 5372c4ee44bdc..931bce91d7d43 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -617,14 +617,12 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { _ => format!("{}", ident), }; - // Assume this is the easy case of `use issue_59764::foo::makro;` and just remove - // intermediate segments. - let (mut span, mut correction) = (directive.span, - format!("{}::{}", module_name, import)); - - if directive.is_nested() { - span = directive.use_span; - + let mut corrections: Vec<(Span, String)> = Vec::new(); + if !directive.is_nested() { + // Assume this is the easy case of `use issue_59764::foo::makro;` and just remove + // intermediate segments. + corrections.push((directive.span, format!("{}::{}", module_name, import))); + } else { // Find the binding span (and any trailing commas and spaces). // ie. `use a::b::{c, d, e};` // ^^^ @@ -652,6 +650,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } debug!("check_for_module_export_macro: removal_span={:?}", removal_span); + // Remove the `removal_span`. + corrections.push((removal_span, "".to_string())); + // Find the span after the crate name and if it has nested imports immediatately // after the crate name already. // ie. `use a::b::{c, d};` @@ -666,34 +667,32 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let source_map = self.resolver.session.source_map(); - // Remove two bytes at the end to keep all but the `};` characters. - // ie. `{b::{c, d}, e::{f, g}};` - // ^^^^^^^^^^^^^^^^^^^^^ - let end_bytes = BytePos(if has_nested { 2 } else { 1 }); - let mut remaining_span = after_crate_name.with_hi( - after_crate_name.hi() - end_bytes); - if has_nested { - // Remove two bytes at the start to keep all but the initial `{` character. - // ie. `{b::{c, d}, e::{f, g}` - // ^^^^^^^^^^^^^^^^^^^^ - remaining_span = remaining_span.with_lo(after_crate_name.lo() + BytePos(1)); + // Add the import to the start, with a `{` if required. + let start_point = source_map.start_point(after_crate_name); + if let Ok(start_snippet) = source_map.span_to_snippet(start_point) { + corrections.push(( + start_point, + if has_nested { + // In this case, `start_snippet` must equal '{'. + format!("{}{}, ", start_snippet, import) + } else { + // In this case, add a `{`, then the moved import, then whatever + // was there before. + format!("{{{}, {}", import, start_snippet) + } + )); } - // Calculate the number of characters into a snippet to remove the removal - // span. - let lo = removal_span.lo() - remaining_span.lo(); - let hi = lo + (removal_span.hi() - removal_span.lo()); - if let Ok(mut remaining) = source_map.span_to_snippet(remaining_span) { - // Remove the original location of the binding. - remaining.replace_range((lo.0 as usize)..(hi.0 as usize), ""); - correction = format!("use {}::{{{}, {}}};", module_name, import, remaining); + // Add a `};` to the end if nested, matching the `{` added at the start. + if !has_nested { + corrections.push((source_map.end_point(after_crate_name), + "};".to_string())); } } let suggestion = Some(( - span, + corrections, String::from("a macro with this name exists at the root of the crate"), - correction, Applicability::MaybeIncorrect, )); let note = vec![ diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index b33356c2ba7ae..c1353d771c26f 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -139,8 +139,8 @@ impl Ord for BindingError { } } -/// A span, message, replacement text, and applicability. -type Suggestion = (Span, String, String, Applicability); +/// A vector of spans and replacements, a message and applicability. +type Suggestion = (Vec<(Span, String)>, String, Applicability); enum ResolutionError<'a> { /// Error E0401: can't use type or const parameters from outer function. @@ -390,8 +390,8 @@ fn resolve_struct_error<'sess, 'a>(resolver: &'sess Resolver<'_>, "failed to resolve: {}", &label); err.span_label(span, label); - if let Some((span, msg, suggestion, applicability)) = suggestion { - err.span_suggestion(span, &msg, suggestion, applicability); + if let Some((suggestions, msg, applicability)) = suggestion { + err.multipart_suggestion(&msg, suggestions, applicability); } err @@ -3774,9 +3774,8 @@ impl<'a> Resolver<'a> { ( String::from("unresolved import"), Some(( - ident.span, + vec![(ident.span, candidate.path.to_string())], String::from("a similar path exists"), - candidate.path.to_string(), Applicability::MaybeIncorrect, )), ) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index ef5f0f54ee369..62af6e19603c4 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -793,8 +793,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { diag.span_label(err.span, label); } - if let Some((span, msg, suggestion, applicability)) = err.suggestion { - diag.span_suggestion(span, &msg, suggestion, applicability); + if let Some((suggestions, msg, applicability)) = err.suggestion { + diag.multipart_suggestion(&msg, suggestions, applicability); } } @@ -947,9 +947,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { label: None, note, suggestion: Some(( - span, + vec![(span, Segment::names_to_string(&suggestion))], String::from("a similar path exists"), - Segment::names_to_string(&suggestion), Applicability::MaybeIncorrect, )), } @@ -1113,8 +1112,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let lev_suggestion = find_best_match_for_name(names, &ident.as_str(), None) .map(|suggestion| - (ident.span, String::from("a similar name exists in the module"), - suggestion.to_string(), Applicability::MaybeIncorrect) + (vec![(ident.span, suggestion.to_string())], + String::from("a similar name exists in the module"), + Applicability::MaybeIncorrect) ); let (suggestion, note) = match self.check_for_module_export_macro( diff --git a/src/test/ui/issue-59764.fixed b/src/test/ui/issue-59764.fixed deleted file mode 100644 index ad3bab50cfca3..0000000000000 --- a/src/test/ui/issue-59764.fixed +++ /dev/null @@ -1,134 +0,0 @@ -// aux-build:issue-59764.rs -// compile-flags:--extern issue_59764 -// edition:2018 -// run-rustfix - -#![allow(warnings)] - -// This tests the suggestion to import macros from the root of a crate. This aims to capture -// the case where a user attempts to import a macro from the definition location instead of the -// root of the crate and the macro is annotated with `#![macro_export]`. - -// Edge cases.. - -mod multiple_imports_same_line_at_end { - use issue_59764::{makro, foo::{baz}}; - //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] -} - -mod multiple_imports_multiline_at_end_trailing_comma { - use issue_59764::{makro, foo::{ - baz, - //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] - }}; -} - -mod multiple_imports_multiline_at_end { - use issue_59764::{makro, foo::{ - baz, - //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] - }}; -} - -mod multiple_imports_same_line_in_middle { - use issue_59764::{makro, foo::{baz, foobar}}; - //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] -} - -mod multiple_imports_multiline_in_middle_trailing_comma { - use issue_59764::{makro, foo::{ - baz, - //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] - foobar, - }}; -} - -mod multiple_imports_multiline_in_middle { - use issue_59764::{makro, foo::{ - baz, - //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] - foobar - }}; -} - -mod nested_imports { - use issue_59764::{makro, foobaz}; - //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] -} - -mod nested_multiple_imports { - use issue_59764::{makro, foobaz, foo::{baz}}; - //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] -} - -mod nested_multiline_multiple_imports_trailing_comma { - use issue_59764::{makro, - foobaz, - foo::{ - baz, - //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] - }, - }; -} - -mod nested_multiline_multiple_imports { - use issue_59764::{makro, - foobaz, - foo::{ - baz, - //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] - } - }; -} - -mod doubly_nested_multiple_imports { - use issue_59764::{makro, foobaz, foo::{baz, barbaz::{barfoo}}}; - //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] -} - -mod doubly_multiline_nested_multiple_imports { - use issue_59764::{makro, - foobaz, - foo::{ - baz, - //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] - barbaz::{ - barfoo, - } - } - }; -} - -mod renamed_import { - use issue_59764::makro as baz; - //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] -} - -mod renamed_multiple_imports { - use issue_59764::{makro as foobar, foo::{baz}}; - //~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] -} - -mod lots_of_whitespace { - use issue_59764::{makro as foobar, - - foobaz, - - - foo::{baz} //~ ERROR unresolved import `issue_59764::foo::makro` [E0432] - - }; -} - -// Simple case.. - -use issue_59764::makro; -//~^ ERROR unresolved import `issue_59764::foo::makro` [E0432] - -makro!(bar); -//~^ ERROR cannot determine resolution for the macro `makro` - -fn main() { - bar(); - //~^ ERROR cannot find function `bar` in this scope [E0425] -} diff --git a/src/test/ui/issue-59764.rs b/src/test/ui/issue-59764.rs index 0c6787691de0a..09dee8c273268 100644 --- a/src/test/ui/issue-59764.rs +++ b/src/test/ui/issue-59764.rs @@ -1,7 +1,6 @@ // aux-build:issue-59764.rs // compile-flags:--extern issue_59764 // edition:2018 -// run-rustfix #![allow(warnings)] diff --git a/src/test/ui/issue-59764.stderr b/src/test/ui/issue-59764.stderr index 89b1f84605dc7..924e69f5f9703 100644 --- a/src/test/ui/issue-59764.stderr +++ b/src/test/ui/issue-59764.stderr @@ -1,5 +1,5 @@ error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:15:33 + --> $DIR/issue-59764.rs:14:33 | LL | use issue_59764::foo::{baz, makro}; | ^^^^^ no `makro` in `foo` @@ -8,10 +8,10 @@ LL | use issue_59764::foo::{baz, makro}; help: a macro with this name exists at the root of the crate | LL | use issue_59764::{makro, foo::{baz}}; - | + | ^^^^^^^^^ --^^ error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:22:9 + --> $DIR/issue-59764.rs:21:9 | LL | makro, | ^^^^^ no `makro` in `foo` @@ -26,7 +26,7 @@ LL | }}; | error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:29:9 + --> $DIR/issue-59764.rs:28:9 | LL | makro | ^^^^^ no `makro` in `foo` @@ -41,7 +41,7 @@ LL | }}; | error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:34:33 + --> $DIR/issue-59764.rs:33:33 | LL | use issue_59764::foo::{baz, makro, foobar}; | ^^^^^ no `makro` in `foo` @@ -50,10 +50,10 @@ LL | use issue_59764::foo::{baz, makro, foobar}; help: a macro with this name exists at the root of the crate | LL | use issue_59764::{makro, foo::{baz, foobar}}; - | + | ^^^^^^^^^ -- ^^ error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:41:9 + --> $DIR/issue-59764.rs:40:9 | LL | makro, | ^^^^^ no `makro` in `foo` @@ -69,7 +69,7 @@ LL | }}; | error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:49:9 + --> $DIR/issue-59764.rs:48:9 | LL | makro, | ^^^^^ no `makro` in `foo` @@ -85,7 +85,7 @@ LL | }}; | error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:55:31 + --> $DIR/issue-59764.rs:54:31 | LL | use issue_59764::{foobaz, foo::makro}; | ^^^^^^^^^^ no `makro` in `foo` @@ -94,10 +94,10 @@ LL | use issue_59764::{foobaz, foo::makro}; help: a macro with this name exists at the root of the crate | LL | use issue_59764::{makro, foobaz}; - | + | ^^^^^^^ -- error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:60:42 + --> $DIR/issue-59764.rs:59:42 | LL | use issue_59764::{foobaz, foo::{baz, makro}}; | ^^^^^ no `makro` in `foo` @@ -106,10 +106,10 @@ LL | use issue_59764::{foobaz, foo::{baz, makro}}; help: a macro with this name exists at the root of the crate | LL | use issue_59764::{makro, foobaz, foo::{baz}}; - | + | ^^^^^^^ -- error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:69:13 + --> $DIR/issue-59764.rs:68:13 | LL | makro, | ^^^^^ no `makro` in `foo` @@ -122,11 +122,10 @@ LL | foobaz, LL | foo::{ LL | baz, LL | -LL | }, - ... + | error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:79:13 + --> $DIR/issue-59764.rs:78:13 | LL | makro | ^^^^^ no `makro` in `foo` @@ -139,11 +138,10 @@ LL | foobaz, LL | foo::{ LL | baz, LL | -LL | } - ... + | error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:85:42 + --> $DIR/issue-59764.rs:84:42 | LL | use issue_59764::{foobaz, foo::{baz, makro, barbaz::{barfoo}}}; | ^^^^^ no `makro` in `foo` @@ -152,10 +150,10 @@ LL | use issue_59764::{foobaz, foo::{baz, makro, barbaz::{barfoo}}}; help: a macro with this name exists at the root of the crate | LL | use issue_59764::{makro, foobaz, foo::{baz, barbaz::{barfoo}}}; - | + | ^^^^^^^ -- error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:94:13 + --> $DIR/issue-59764.rs:93:13 | LL | makro, | ^^^^^ no `makro` in `foo` @@ -168,11 +166,10 @@ LL | foobaz, LL | foo::{ LL | baz, LL | -LL | barbaz::{ - ... + | error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:103:9 + --> $DIR/issue-59764.rs:102:9 | LL | use issue_59764::foo::makro as baz; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo` @@ -184,7 +181,7 @@ LL | use issue_59764::makro as baz; | ^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:108:33 + --> $DIR/issue-59764.rs:107:33 | LL | use issue_59764::foo::{baz, makro as foobar}; | ^^^^^^^^^^^^^^^ no `makro` in `foo` @@ -193,10 +190,10 @@ LL | use issue_59764::foo::{baz, makro as foobar}; help: a macro with this name exists at the root of the crate | LL | use issue_59764::{makro as foobar, foo::{baz}}; - | + | ^^^^^^^^^^^^^^^^^^^ --^^ error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:121:17 + --> $DIR/issue-59764.rs:120:17 | LL | makro as foobar} | ^^^^^^^^^^^^^^^ no `makro` in `foo` @@ -204,16 +201,16 @@ LL | makro as foobar} = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined help: a macro with this name exists at the root of the crate | -LL | use issue_59764::{makro as foobar, +LL | issue_59764::{makro as foobar, LL | LL | foobaz, LL | LL | LL | foo::{baz} - ... + | error[E0432]: unresolved import `issue_59764::foo::makro` - --> $DIR/issue-59764.rs:128:5 + --> $DIR/issue-59764.rs:127:5 | LL | use issue_59764::foo::makro; | ^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo` @@ -225,7 +222,7 @@ LL | use issue_59764::makro; | ^^^^^^^^^^^^^^^^^^ error: cannot determine resolution for the macro `makro` - --> $DIR/issue-59764.rs:131:1 + --> $DIR/issue-59764.rs:130:1 | LL | makro!(bar); | ^^^^^ @@ -233,7 +230,7 @@ LL | makro!(bar); = note: import resolution is stuck, try simplifying macro imports error[E0425]: cannot find function `bar` in this scope - --> $DIR/issue-59764.rs:135:5 + --> $DIR/issue-59764.rs:134:5 | LL | bar(); | ^^^ not found in this scope From 633fc9eef0d26a1c9b59fad8e807632f234a8aae Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 12 Apr 2019 12:30:21 +0200 Subject: [PATCH 16/23] Revert PR #59401 to fix issue #59652 (a stable-to-beta regression). This is result of squashing two revert commits: Revert "compile all crates under test w/ -Zemit-stack-sizes" This reverts commit 7d365cf27f4249fc9b61ba8abfc813abe43f1cb7. Revert "bootstrap: build compiler-builtins with -Z emit-stack-sizes" This reverts commit 8b8488ce8fc047282e7159343f30609417f9fa39. --- src/bootstrap/bin/rustc.rs | 27 --------------------------- src/bootstrap/compile.rs | 4 ---- 2 files changed, 31 deletions(-) diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index 86ce5fd01a812..a76584093fc76 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -187,33 +187,6 @@ fn main() { cmd.arg("-C").arg(format!("debug-assertions={}", debug_assertions)); } - // Build all crates in the `std` facade with `-Z emit-stack-sizes` to add stack usage - // information. - // - // When you use this `-Z` flag with Cargo you get stack usage information on all crates - // compiled from source, and when you are using LTO you also get information on pre-compiled - // crates like `core` and `std`, even if they were not compiled with `-Z emit-stack-sizes`. - // However, there's an exception: `compiler_builtins`. This crate is special and doesn't - // participate in LTO because it's always linked as a separate object file. For this reason - // it's impossible to get stack usage information about `compiler-builtins` using - // `RUSTFLAGS` + Cargo, or `cargo rustc`. - // - // To make the stack usage information of all crates under the `std` facade available to - // Cargo based stack usage analysis tools, in both LTO and non-LTO mode, we compile them - // with the `-Z emit-stack-sizes` flag. The `RUSTC_EMIT_STACK_SIZES` var helps us apply this - // flag only to the crates in the `std` facade. The `-Z` flag is known to currently work - // with targets that produce ELF files so we limit its use flag to those targets. - // - // NOTE(japaric) if this ever causes problem with an LLVM upgrade or any PR feel free to - // remove it or comment it out - if env::var_os("RUSTC_EMIT_STACK_SIZES").is_some() - && (target.contains("-linux-") - || target.contains("-none-eabi") - || target.ends_with("-none-elf")) - { - cmd.arg("-Zemit-stack-sizes"); - } - if let Ok(s) = env::var("RUSTC_CODEGEN_UNITS") { cmd.arg("-C").arg(format!("codegen-units={}", s)); } diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 08316b71ea85b..66443d472d334 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -97,8 +97,6 @@ impl Step for Std { let _folder = builder.fold_output(|| format!("stage{}-std", compiler.stage)); builder.info(&format!("Building stage{} std artifacts ({} -> {})", compiler.stage, &compiler.host, target)); - // compile with `-Z emit-stack-sizes`; see bootstrap/src/rustc.rs for more details - cargo.env("RUSTC_EMIT_STACK_SIZES", "1"); run_cargo(builder, &mut cargo, &libstd_stamp(builder, compiler, target), @@ -397,8 +395,6 @@ impl Step for Test { let _folder = builder.fold_output(|| format!("stage{}-test", compiler.stage)); builder.info(&format!("Building stage{} test artifacts ({} -> {})", compiler.stage, &compiler.host, target)); - // compile with `-Z emit-stack-sizes`; see bootstrap/src/rustc.rs for more details - cargo.env("RUSTC_EMIT_STACK_SIZES", "1"); run_cargo(builder, &mut cargo, &libtest_stamp(builder, compiler, target), From 796e6e37d6fa76b719dbf9b38872daffdc9caa70 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 11 Apr 2019 22:48:53 +0200 Subject: [PATCH 17/23] Don't generate empty json variables --- src/librustdoc/html/render.rs | 19 +++++++++--- src/librustdoc/html/static/source-script.js | 34 ++++++++++++--------- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 866d8fe682a7b..3f2eaf8ec5527 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -1060,11 +1060,22 @@ themePicker.onblur = handleThemeButtonsBlur; .expect("invalid osstring conversion"))) .collect::>(); files.sort_unstable_by(|a, b| a.cmp(b)); - // FIXME(imperio): we could avoid to generate "dirs" and "files" if they're empty. - format!("{{\"name\":\"{name}\",\"dirs\":[{subs}],\"files\":[{files}]}}", + let subs = subs.iter().map(|s| s.to_json_string()).collect::>().join(","); + let dirs = if subs.is_empty() { + String::new() + } else { + format!(",\"dirs\":[{}]", subs) + }; + let files = files.join(","); + let files = if files.is_empty() { + String::new() + } else { + format!(",\"files\":[{}]", files) + }; + format!("{{\"name\":\"{name}\"{dirs}{files}}}", name=self.elem.to_str().expect("invalid osstring conversion"), - subs=subs.iter().map(|s| s.to_json_string()).collect::>().join(","), - files=files.join(",")) + dirs=dirs, + files=files) } } diff --git a/src/librustdoc/html/static/source-script.js b/src/librustdoc/html/static/source-script.js index 509c628ce5a46..567022b4139ad 100644 --- a/src/librustdoc/html/static/source-script.js +++ b/src/librustdoc/html/static/source-script.js @@ -39,28 +39,32 @@ function createDirEntry(elem, parent, fullPath, currentFile, hasFoundFile) { children.className = "children"; var folders = document.createElement("div"); folders.className = "folders"; - for (var i = 0; i < elem.dirs.length; ++i) { - if (createDirEntry(elem.dirs[i], folders, fullPath, currentFile, - hasFoundFile) === true) { - addClass(name, "expand"); - hasFoundFile = true; + if (elem.dirs) { + for (var i = 0; i < elem.dirs.length; ++i) { + if (createDirEntry(elem.dirs[i], folders, fullPath, currentFile, + hasFoundFile) === true) { + addClass(name, "expand"); + hasFoundFile = true; + } } } children.appendChild(folders); var files = document.createElement("div"); files.className = "files"; - for (i = 0; i < elem.files.length; ++i) { - var file = document.createElement("a"); - file.innerText = elem.files[i]; - file.href = window.rootPath + "src/" + fullPath + elem.files[i] + ".html"; - if (hasFoundFile === false && - currentFile === fullPath + elem.files[i]) { - file.className = "selected"; - addClass(name, "expand"); - hasFoundFile = true; + if (elem.files) { + for (i = 0; i < elem.files.length; ++i) { + var file = document.createElement("a"); + file.innerText = elem.files[i]; + file.href = window.rootPath + "src/" + fullPath + elem.files[i] + ".html"; + if (hasFoundFile === false && + currentFile === fullPath + elem.files[i]) { + file.className = "selected"; + addClass(name, "expand"); + hasFoundFile = true; + } + files.appendChild(file); } - files.appendChild(file); } search.fullPath = fullPath; children.appendChild(files); From 4f28431e39fa2abc16703e067bcf9a7e3a0502d4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 7 Apr 2019 18:09:28 +0200 Subject: [PATCH 18/23] Apply resource-suffix to search-index and source-files scripts as well --- src/librustdoc/html/layout.rs | 4 ++-- src/librustdoc/html/render.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs index 6ff3917a265ed..acf019fd2254d 100644 --- a/src/librustdoc/html/layout.rs +++ b/src/librustdoc/html/layout.rs @@ -157,11 +157,11 @@ pub fn render( window.rootPath = \"{root_path}\";\ window.currentCrate = \"{krate}\";\ \ - \ + \ \ {static_extra_scripts}\ {extra_scripts}\ - \ + \ \ ", css_extension = if css_file_extension { diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 866d8fe682a7b..65922dfb24b2c 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -1009,7 +1009,7 @@ themePicker.onblur = handleThemeButtonsBlur; }) } - let dst = cx.dst.join("aliases.js"); + let dst = cx.dst.join(&format!("aliases{}.js", cx.shared.resource_suffix)); { let (mut all_aliases, _, _) = try_err!(collect(&dst, &krate.name, "ALIASES", false), &dst); let mut w = try_err!(File::create(&dst), &dst); @@ -1097,7 +1097,7 @@ themePicker.onblur = handleThemeButtonsBlur; } } - let dst = cx.dst.join("source-files.js"); + let dst = cx.dst.join(&format!("source-files{}.js", cx.shared.resource_suffix)); let (mut all_sources, _krates, _) = try_err!(collect(&dst, &krate.name, "sourcesIndex", false), &dst); @@ -1113,7 +1113,7 @@ themePicker.onblur = handleThemeButtonsBlur; } // Update the search index - let dst = cx.dst.join("search-index.js"); + let dst = cx.dst.join(&format!("search-index{}.js", cx.shared.resource_suffix)); let (mut all_indexes, mut krates, variables) = try_err!(collect(&dst, &krate.name, "searchIndex", @@ -1476,7 +1476,7 @@ impl<'a> SourceCollector<'a> { description: &desc, keywords: BASIC_KEYWORDS, resource_suffix: &self.scx.resource_suffix, - extra_scripts: &["source-files"], + extra_scripts: &[&format!("source-files{}", self.scx.resource_suffix)], static_extra_scripts: &[&format!("source-script{}", self.scx.resource_suffix)], }; layout::render(&mut w, &self.scx.layout, From 7e620529e3f3be464e18173b495222ea90b31ccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 3 Apr 2019 02:43:49 +0200 Subject: [PATCH 19/23] Use a proc macro to declare preallocated symbols --- Cargo.lock | 1 + src/librustc_macros/src/lib.rs | 6 + src/librustc_macros/src/symbols.rs | 173 +++++++++++++++++++++++ src/libsyntax/ast.rs | 6 + src/libsyntax/attr/mod.rs | 21 +++ src/libsyntax/feature_gate.rs | 28 ++-- src/libsyntax_pos/Cargo.toml | 1 + src/libsyntax_pos/lib.rs | 2 + src/libsyntax_pos/symbol.rs | 213 ++++++++++++----------------- 9 files changed, 312 insertions(+), 139 deletions(-) create mode 100644 src/librustc_macros/src/symbols.rs diff --git a/Cargo.lock b/Cargo.lock index 8fd365b8462b7..c57a72d67bfcb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3374,6 +3374,7 @@ dependencies = [ "arena 0.0.0", "cfg-if 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_data_structures 0.0.0", + "rustc_macros 0.1.0", "scoped-tls 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "serialize 0.0.0", "unicode-width 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/src/librustc_macros/src/lib.rs b/src/librustc_macros/src/lib.rs index e99ceb1b0c79b..98fba55218f9b 100644 --- a/src/librustc_macros/src/lib.rs +++ b/src/librustc_macros/src/lib.rs @@ -9,10 +9,16 @@ use proc_macro::TokenStream; mod hash_stable; mod query; +mod symbols; #[proc_macro] pub fn rustc_queries(input: TokenStream) -> TokenStream { query::rustc_queries(input) } +#[proc_macro] +pub fn symbols(input: TokenStream) -> TokenStream { + symbols::symbols(input) +} + decl_derive!([HashStable, attributes(stable_hasher)] => hash_stable::hash_stable_derive); diff --git a/src/librustc_macros/src/symbols.rs b/src/librustc_macros/src/symbols.rs new file mode 100644 index 0000000000000..e72ab7f84e92a --- /dev/null +++ b/src/librustc_macros/src/symbols.rs @@ -0,0 +1,173 @@ +use proc_macro::TokenStream; +use syn::{ + Token, Ident, LitStr, + braced, parse_macro_input, +}; +use syn::parse::{Result, Parse, ParseStream}; +use syn; +use std::collections::HashSet; +use quote::quote; + +#[allow(non_camel_case_types)] +mod kw { + syn::custom_keyword!(Keywords); + syn::custom_keyword!(Other); +} + +struct Keyword { + name: Ident, + value: LitStr, +} + +impl Parse for Keyword { + fn parse(input: ParseStream<'_>) -> Result { + let name = input.parse()?; + input.parse::()?; + let value = input.parse()?; + input.parse::()?; + + Ok(Keyword { + name, + value, + }) + } +} + +struct Symbol(Ident); + +impl Parse for Symbol { + fn parse(input: ParseStream<'_>) -> Result { + let ident: Ident = input.parse()?; + input.parse::()?; + + Ok(Symbol(ident)) + } +} + +/// A type used to greedily parse another type until the input is empty. +struct List(Vec); + +impl Parse for List { + fn parse(input: ParseStream<'_>) -> Result { + let mut list = Vec::new(); + while !input.is_empty() { + list.push(input.parse()?); + } + Ok(List(list)) + } +} + +struct Input { + keywords: List, + symbols: List, +} + +impl Parse for Input { + fn parse(input: ParseStream<'_>) -> Result { + input.parse::()?; + let content; + braced!(content in input); + let keywords = content.parse()?; + + input.parse::()?; + let content; + braced!(content in input); + let symbols = content.parse()?; + + Ok(Input { + keywords, + symbols, + }) + } +} + +pub fn symbols(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as Input); + + let mut keyword_stream = quote! {}; + let mut symbols_stream = quote! {}; + let mut prefill_stream = quote! {}; + let mut from_str_stream = quote! {}; + let mut counter = 0u32; + let mut keys = HashSet::::new(); + + let mut check_dup = |str: &str| { + if !keys.insert(str.to_string()) { + panic!("Symbol `{}` is duplicated", str); + } + }; + + for keyword in &input.keywords.0 { + let name = &keyword.name; + let value = &keyword.value; + check_dup(&value.value()); + prefill_stream.extend(quote! { + #value, + }); + keyword_stream.extend(quote! { + pub const #name: Keyword = Keyword { + ident: Ident::with_empty_ctxt(super::Symbol::new(#counter)) + }; + }); + from_str_stream.extend(quote! { + #value => Ok(#name), + }); + counter += 1; + } + + for symbol in &input.symbols.0 { + let value = &symbol.0; + let value_str = value.to_string(); + check_dup(&value_str); + prefill_stream.extend(quote! { + #value_str, + }); + symbols_stream.extend(quote! { + pub const #value: Symbol = Symbol::new(#counter); + }); + counter += 1; + } + + TokenStream::from(quote! { + #[allow(non_upper_case_globals)] + pub mod keywords { + use super::{Symbol, Ident}; + #[derive(Clone, Copy, PartialEq, Eq)] + pub struct Keyword { + ident: Ident, + } + impl Keyword { + #[inline] pub fn ident(self) -> Ident { self.ident } + #[inline] pub fn name(self) -> Symbol { self.ident.name } + } + + #keyword_stream + + impl std::str::FromStr for Keyword { + type Err = (); + + fn from_str(s: &str) -> Result { + match s { + #from_str_stream + _ => Err(()), + } + } + } + } + + #[allow(non_upper_case_globals)] + pub mod symbols { + use super::Symbol; + + #symbols_stream + } + + impl Interner { + pub fn fresh() -> Self { + Interner::prefill(&[ + #prefill_stream + ]) + } + } + }) +} diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index bcc8fdf8cd4e7..735b4ef4a3e52 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -68,6 +68,12 @@ pub struct Path { pub segments: Vec, } +impl PartialEq for Path { + fn eq(&self, symbol: &Symbol) -> bool { + self.segments.len() == 1 && self.segments[0].ident.name.interned() == *symbol + } +} + impl<'a> PartialEq<&'a str> for Path { fn eq(&self, string: &&'a str) -> bool { self.segments.len() == 1 && self.segments[0].ident.name == *string diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index c0bd5c79b1dd1..f34bbc9f35dad 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -85,6 +85,11 @@ impl NestedMetaItem { self.meta_item().map_or(false, |meta_item| meta_item.check_name(name)) } + /// Returns `true` if this list item is a MetaItem with a name of `name`. + pub fn check_name_symbol(&self, name: Symbol) -> bool { + self.meta_item().map_or(false, |meta_item| meta_item.check_name_symbol(name)) + } + /// For a single-segment meta-item returns its name, otherwise returns `None`. pub fn ident(&self) -> Option { self.meta_item().and_then(|meta_item| meta_item.ident()) @@ -159,6 +164,18 @@ impl Attribute { matches } + /// Returns `true` if the attribute's path matches the argument. If it matches, then the + /// attribute is marked as used. + /// + /// To check the attribute name without marking it used, use the `path` field directly. + pub fn check_name_symbol(&self, name: Symbol) -> bool { + let matches = self.path == name; + if matches { + mark_used(self); + } + matches + } + /// For a single-segment attribute returns its name, otherwise returns `None`. pub fn ident(&self) -> Option { if self.path.segments.len() == 1 { @@ -248,6 +265,10 @@ impl MetaItem { self.path == name } + pub fn check_name_symbol(&self, name: Symbol) -> bool { + self.path == name + } + pub fn is_value_str(&self) -> bool { self.value_str().is_some() } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index dcb55fb572f33..9f655c4158008 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -28,7 +28,7 @@ use crate::tokenstream::TokenTree; use errors::{DiagnosticBuilder, Handler}; use rustc_data_structures::fx::FxHashMap; use rustc_target::spec::abi::Abi; -use syntax_pos::{Span, DUMMY_SP}; +use syntax_pos::{Span, DUMMY_SP, symbols}; use log::debug; use std::env; @@ -1366,7 +1366,7 @@ impl<'a> Context<'a> { } } else if n == "doc" { if let Some(content) = attr.meta_item_list() { - if content.iter().any(|c| c.check_name("include")) { + if content.iter().any(|c| c.check_name_symbol(symbols::include)) { gate_feature!(self, external_doc, attr.span, "#[doc(include = \"...\")] is experimental" ); @@ -1648,25 +1648,25 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { // check for gated attributes self.context.check_attribute(attr, false); - if attr.check_name("doc") { + if attr.check_name_symbol(symbols::doc) { if let Some(content) = attr.meta_item_list() { - if content.len() == 1 && content[0].check_name("cfg") { + if content.len() == 1 && content[0].check_name_symbol(symbols::cfg) { gate_feature_post!(&self, doc_cfg, attr.span, "#[doc(cfg(...))] is experimental" ); - } else if content.iter().any(|c| c.check_name("masked")) { + } else if content.iter().any(|c| c.check_name_symbol(symbols::masked)) { gate_feature_post!(&self, doc_masked, attr.span, "#[doc(masked)] is experimental" ); - } else if content.iter().any(|c| c.check_name("spotlight")) { + } else if content.iter().any(|c| c.check_name_symbol(symbols::spotlight)) { gate_feature_post!(&self, doc_spotlight, attr.span, "#[doc(spotlight)] is experimental" ); - } else if content.iter().any(|c| c.check_name("alias")) { + } else if content.iter().any(|c| c.check_name_symbol(symbols::alias)) { gate_feature_post!(&self, doc_alias, attr.span, "#[doc(alias = \"...\")] is experimental" ); - } else if content.iter().any(|c| c.check_name("keyword")) { + } else if content.iter().any(|c| c.check_name_symbol(symbols::keyword)) { gate_feature_post!(&self, doc_keyword, attr.span, "#[doc(keyword = \"...\")] is experimental" ); @@ -1674,7 +1674,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { } } - match BUILTIN_ATTRIBUTES.iter().find(|(name, ..)| attr.path == name) { + match BUILTIN_ATTRIBUTES.iter().find(|(name, ..)| attr.path == *name) { Some(&(name, _, template, _)) => self.check_builtin_attribute(attr, name, template), None => if let Some(TokenTree::Token(_, token::Eq)) = attr.tokens.trees().next() { // All key-value attributes are restricted to meta-item syntax. @@ -1727,7 +1727,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { ast::ItemKind::Struct(..) => { for attr in attr::filter_by_name(&i.attrs[..], "repr") { for item in attr.meta_item_list().unwrap_or_else(Vec::new) { - if item.check_name("simd") { + if item.check_name_symbol(symbols::simd) { gate_feature_post!(&self, repr_simd, attr.span, "SIMD types are experimental and possibly buggy"); } @@ -1738,7 +1738,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { ast::ItemKind::Enum(..) => { for attr in attr::filter_by_name(&i.attrs[..], "repr") { for item in attr.meta_item_list().unwrap_or_else(Vec::new) { - if item.check_name("align") { + if item.check_name_symbol(symbols::align) { gate_feature_post!(&self, repr_align_enum, attr.span, "`#[repr(align(x))]` on enums is experimental"); } @@ -2062,7 +2062,7 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute], // Process the edition umbrella feature-gates first, to ensure // `edition_enabled_features` is completed before it's queried. for attr in krate_attrs { - if !attr.check_name("feature") { + if !attr.check_name_symbol(symbols::feature) { continue } @@ -2107,7 +2107,7 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute], } for attr in krate_attrs { - if !attr.check_name("feature") { + if !attr.check_name_symbol(symbols::feature) { continue } @@ -2237,7 +2237,7 @@ fn maybe_stage_features(span_handler: &Handler, krate: &ast::Crate, }; if !allow_features { for attr in &krate.attrs { - if attr.check_name("feature") { + if attr.check_name_symbol(symbols::feature) { let release_channel = option_env!("CFG_RELEASE_CHANNEL").unwrap_or("(unknown)"); span_err!(span_handler, attr.span, E0554, "#![feature] may not be used on the {} release channel", diff --git a/src/libsyntax_pos/Cargo.toml b/src/libsyntax_pos/Cargo.toml index 691abffbbc1af..af7edc0a6bd3e 100644 --- a/src/libsyntax_pos/Cargo.toml +++ b/src/libsyntax_pos/Cargo.toml @@ -11,6 +11,7 @@ crate-type = ["dylib"] [dependencies] serialize = { path = "../libserialize" } +rustc_macros = { path = "../librustc_macros" } rustc_data_structures = { path = "../librustc_data_structures" } arena = { path = "../libarena" } scoped-tls = "1.0" diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index db1543ff13f7e..da857abe66e23 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -16,6 +16,7 @@ #![feature(non_exhaustive)] #![feature(optin_builtin_traits)] #![feature(rustc_attrs)] +#![feature(proc_macro_hygiene)] #![feature(specialization)] #![feature(step_trait)] @@ -32,6 +33,7 @@ mod span_encoding; pub use span_encoding::{Span, DUMMY_SP}; pub mod symbol; +pub use symbol::symbols; mod analyze_source_file; diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index 393f52e7de517..0b00d4f9a54b1 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -6,6 +6,7 @@ use arena::DroplessArena; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::newtype_index; +use rustc_macros::symbols; use serialize::{Decodable, Decoder, Encodable, Encoder}; use std::fmt; @@ -16,6 +17,93 @@ use std::hash::{Hash, Hasher}; use crate::hygiene::SyntaxContext; use crate::{Span, DUMMY_SP, GLOBALS}; +symbols! { + // After modifying this list adjust `is_special`, `is_used_keyword`/`is_unused_keyword`, + // this should be rarely necessary though if the keywords are kept in alphabetic order. + Keywords { + // Special reserved identifiers used internally for elided lifetimes, + // unnamed method parameters, crate root module, error recovery etc. + Invalid, "", + PathRoot, "{{root}}", + DollarCrate, "$crate", + Underscore, "_", + + // Keywords that are used in stable Rust. + As, "as", + Box, "box", + Break, "break", + Const, "const", + Continue, "continue", + Crate, "crate", + Else, "else", + Enum, "enum", + Extern, "extern", + False, "false", + Fn, "fn", + For, "for", + If, "if", + Impl, "impl", + In, "in", + Let, "let", + Loop, "loop", + Match, "match", + Mod, "mod", + Move, "move", + Mut, "mut", + Pub, "pub", + Ref, "ref", + Return, "return", + SelfLower, "self", + SelfUpper, "Self", + Static, "static", + Struct, "struct", + Super, "super", + Trait, "trait", + True, "true", + Type, "type", + Unsafe, "unsafe", + Use, "use", + Where, "where", + While, "while", + + // Keywords that are used in unstable Rust or reserved for future use. + Abstract, "abstract", + Become, "become", + Do, "do", + Final, "final", + Macro, "macro", + Override, "override", + Priv, "priv", + Typeof, "typeof", + Unsized, "unsized", + Virtual, "virtual", + Yield, "yield", + + // Edition-specific keywords that are used in stable Rust. + Dyn, "dyn", // >= 2018 Edition only + + // Edition-specific keywords that are used in unstable Rust or reserved for future use. + Async, "async", // >= 2018 Edition only + Try, "try", // >= 2018 Edition only + + // Special lifetime names + UnderscoreLifetime, "'_", + StaticLifetime, "'static", + + // Weak keywords, have special meaning only in specific contexts. + Auto, "auto", + Catch, "catch", + Default, "default", + Existential, "existential", + Union, "union", + } + + // Other symbols that can be referred to with syntax_pos::symbols::* + Other { + doc, cfg, masked, spotlight, alias, keyword, feature, include, simd, align, + } +} + #[derive(Copy, Clone, Eq)] pub struct Ident { pub name: Symbol, @@ -317,131 +405,6 @@ impl Interner { } } -// In this macro, there is the requirement that the name (the number) must be monotonically -// increasing by one in the special identifiers, starting at 0; the same holds for the keywords, -// except starting from the next number instead of zero. -macro_rules! declare_keywords {( - $( ($index: expr, $konst: ident, $string: expr) )* -) => { - pub mod keywords { - use super::{Symbol, Ident}; - #[derive(Clone, Copy, PartialEq, Eq)] - pub struct Keyword { - ident: Ident, - } - impl Keyword { - #[inline] pub fn ident(self) -> Ident { self.ident } - #[inline] pub fn name(self) -> Symbol { self.ident.name } - } - $( - #[allow(non_upper_case_globals)] - pub const $konst: Keyword = Keyword { - ident: Ident::with_empty_ctxt(super::Symbol::new($index)) - }; - )* - - impl std::str::FromStr for Keyword { - type Err = (); - - fn from_str(s: &str) -> Result { - match s { - $($string => Ok($konst),)* - _ => Err(()), - } - } - } - } - - impl Interner { - pub fn fresh() -> Self { - Interner::prefill(&[$($string,)*]) - } - } -}} - -// N.B., leaving holes in the ident table is bad! a different ident will get -// interned with the id from the hole, but it will be between the min and max -// of the reserved words, and thus tagged as "reserved". -// After modifying this list adjust `is_special`, `is_used_keyword`/`is_unused_keyword`, -// this should be rarely necessary though if the keywords are kept in alphabetic order. -declare_keywords! { - // Special reserved identifiers used internally for elided lifetimes, - // unnamed method parameters, crate root module, error recovery etc. - (0, Invalid, "") - (1, PathRoot, "{{root}}") - (2, DollarCrate, "$crate") - (3, Underscore, "_") - - // Keywords that are used in stable Rust. - (4, As, "as") - (5, Box, "box") - (6, Break, "break") - (7, Const, "const") - (8, Continue, "continue") - (9, Crate, "crate") - (10, Else, "else") - (11, Enum, "enum") - (12, Extern, "extern") - (13, False, "false") - (14, Fn, "fn") - (15, For, "for") - (16, If, "if") - (17, Impl, "impl") - (18, In, "in") - (19, Let, "let") - (20, Loop, "loop") - (21, Match, "match") - (22, Mod, "mod") - (23, Move, "move") - (24, Mut, "mut") - (25, Pub, "pub") - (26, Ref, "ref") - (27, Return, "return") - (28, SelfLower, "self") - (29, SelfUpper, "Self") - (30, Static, "static") - (31, Struct, "struct") - (32, Super, "super") - (33, Trait, "trait") - (34, True, "true") - (35, Type, "type") - (36, Unsafe, "unsafe") - (37, Use, "use") - (38, Where, "where") - (39, While, "while") - - // Keywords that are used in unstable Rust or reserved for future use. - (40, Abstract, "abstract") - (41, Become, "become") - (42, Do, "do") - (43, Final, "final") - (44, Macro, "macro") - (45, Override, "override") - (46, Priv, "priv") - (47, Typeof, "typeof") - (48, Unsized, "unsized") - (49, Virtual, "virtual") - (50, Yield, "yield") - - // Edition-specific keywords that are used in stable Rust. - (51, Dyn, "dyn") // >= 2018 Edition only - - // Edition-specific keywords that are used in unstable Rust or reserved for future use. - (52, Async, "async") // >= 2018 Edition only - (53, Try, "try") // >= 2018 Edition only - - // Special lifetime names - (54, UnderscoreLifetime, "'_") - (55, StaticLifetime, "'static") - - // Weak keywords, have special meaning only in specific contexts. - (56, Auto, "auto") - (57, Catch, "catch") - (58, Default, "default") - (59, Existential, "existential") - (60, Union, "union") -} - impl Symbol { fn is_used_keyword_2018(self) -> bool { self == keywords::Dyn.name() From 0e26063b5b06056f4dc985e501a18da84db6bfbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Fri, 5 Apr 2019 03:05:30 +0200 Subject: [PATCH 20/23] Make check_name generic --- src/librustc/middle/lib_features.rs | 6 ++-- .../persist/dirty_clean.rs | 2 +- src/librustc_lint/unused.rs | 2 +- src/libsyntax/attr/mod.rs | 36 +++++++------------ src/libsyntax/feature_gate.rs | 24 ++++++------- src/libsyntax_ext/proc_macro_decls.rs | 2 +- src/libsyntax_pos/symbol.rs | 3 +- 7 files changed, 32 insertions(+), 43 deletions(-) diff --git a/src/librustc/middle/lib_features.rs b/src/librustc/middle/lib_features.rs index ee8dd9e58b5ff..e79ef8bfc6fb4 100644 --- a/src/librustc/middle/lib_features.rs +++ b/src/librustc/middle/lib_features.rs @@ -8,7 +8,7 @@ use crate::ty::TyCtxt; use crate::hir::intravisit::{self, NestedVisitorMap, Visitor}; use syntax::symbol::Symbol; use syntax::ast::{Attribute, MetaItem, MetaItemKind}; -use syntax_pos::Span; +use syntax_pos::{Span, symbols}; use rustc_data_structures::fx::{FxHashSet, FxHashMap}; use rustc_macros::HashStable; use errors::DiagnosticId; @@ -51,12 +51,12 @@ impl<'a, 'tcx> LibFeatureCollector<'a, 'tcx> { } fn extract(&self, attr: &Attribute) -> Option<(Symbol, Option, Span)> { - let stab_attrs = vec!["stable", "unstable", "rustc_const_unstable"]; + let stab_attrs = [symbols::stable, symbols::unstable, symbols::rustc_const_unstable]; // Find a stability attribute (i.e., `#[stable (..)]`, `#[unstable (..)]`, // `#[rustc_const_unstable (..)]`). if let Some(stab_attr) = stab_attrs.iter().find(|stab_attr| { - attr.check_name(stab_attr) + attr.check_name(**stab_attr) }) { let meta_item = attr.meta(); if let Some(MetaItem { node: MetaItemKind::List(ref metas), .. }) = meta_item { diff --git a/src/librustc_incremental/persist/dirty_clean.rs b/src/librustc_incremental/persist/dirty_clean.rs index 5c4fd9dba7af4..6f5b411946c2d 100644 --- a/src/librustc_incremental/persist/dirty_clean.rs +++ b/src/librustc_incremental/persist/dirty_clean.rs @@ -599,7 +599,7 @@ impl<'a, 'tcx> FindAllAttrs<'a, 'tcx> { fn is_active_attr(&mut self, attr: &Attribute) -> bool { for attr_name in &self.attr_names { - if attr.check_name(attr_name) && check_config(self.tcx, attr) { + if attr.check_name(*attr_name) && check_config(self.tcx, attr) { return true; } } diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index d41d97f58bcbe..2aee21abb58e5 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -228,7 +228,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes { let plugin_attributes = cx.sess().plugin_attributes.borrow_mut(); for &(ref name, ty) in plugin_attributes.iter() { - if ty == AttributeType::Whitelisted && attr.check_name(&name) { + if ty == AttributeType::Whitelisted && attr.check_name(&**name) { debug!("{:?} (plugin attr) is whitelisted with ty {:?}", name, ty); break; } diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index f34bbc9f35dad..e00f91e395280 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -81,15 +81,13 @@ impl NestedMetaItem { } /// Returns `true` if this list item is a MetaItem with a name of `name`. - pub fn check_name(&self, name: &str) -> bool { + pub fn check_name(&self, name: T) -> bool + where + Path: PartialEq, + { self.meta_item().map_or(false, |meta_item| meta_item.check_name(name)) } - /// Returns `true` if this list item is a MetaItem with a name of `name`. - pub fn check_name_symbol(&self, name: Symbol) -> bool { - self.meta_item().map_or(false, |meta_item| meta_item.check_name_symbol(name)) - } - /// For a single-segment meta-item returns its name, otherwise returns `None`. pub fn ident(&self) -> Option { self.meta_item().and_then(|meta_item| meta_item.ident()) @@ -156,19 +154,10 @@ impl Attribute { /// attribute is marked as used. /// /// To check the attribute name without marking it used, use the `path` field directly. - pub fn check_name(&self, name: &str) -> bool { - let matches = self.path == name; - if matches { - mark_used(self); - } - matches - } - - /// Returns `true` if the attribute's path matches the argument. If it matches, then the - /// attribute is marked as used. - /// - /// To check the attribute name without marking it used, use the `path` field directly. - pub fn check_name_symbol(&self, name: Symbol) -> bool { + pub fn check_name(&self, name: T) -> bool + where + Path: PartialEq, + { let matches = self.path == name; if matches { mark_used(self); @@ -261,11 +250,10 @@ impl MetaItem { } } - pub fn check_name(&self, name: &str) -> bool { - self.path == name - } - - pub fn check_name_symbol(&self, name: Symbol) -> bool { + pub fn check_name(&self, name: T) -> bool + where + Path: PartialEq, + { self.path == name } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 9f655c4158008..80aeb31337d50 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -1366,7 +1366,7 @@ impl<'a> Context<'a> { } } else if n == "doc" { if let Some(content) = attr.meta_item_list() { - if content.iter().any(|c| c.check_name_symbol(symbols::include)) { + if content.iter().any(|c| c.check_name(symbols::include)) { gate_feature!(self, external_doc, attr.span, "#[doc(include = \"...\")] is experimental" ); @@ -1648,25 +1648,25 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { // check for gated attributes self.context.check_attribute(attr, false); - if attr.check_name_symbol(symbols::doc) { + if attr.check_name(symbols::doc) { if let Some(content) = attr.meta_item_list() { - if content.len() == 1 && content[0].check_name_symbol(symbols::cfg) { + if content.len() == 1 && content[0].check_name(symbols::cfg) { gate_feature_post!(&self, doc_cfg, attr.span, "#[doc(cfg(...))] is experimental" ); - } else if content.iter().any(|c| c.check_name_symbol(symbols::masked)) { + } else if content.iter().any(|c| c.check_name(symbols::masked)) { gate_feature_post!(&self, doc_masked, attr.span, "#[doc(masked)] is experimental" ); - } else if content.iter().any(|c| c.check_name_symbol(symbols::spotlight)) { + } else if content.iter().any(|c| c.check_name(symbols::spotlight)) { gate_feature_post!(&self, doc_spotlight, attr.span, "#[doc(spotlight)] is experimental" ); - } else if content.iter().any(|c| c.check_name_symbol(symbols::alias)) { + } else if content.iter().any(|c| c.check_name(symbols::alias)) { gate_feature_post!(&self, doc_alias, attr.span, "#[doc(alias = \"...\")] is experimental" ); - } else if content.iter().any(|c| c.check_name_symbol(symbols::keyword)) { + } else if content.iter().any(|c| c.check_name(symbols::keyword)) { gate_feature_post!(&self, doc_keyword, attr.span, "#[doc(keyword = \"...\")] is experimental" ); @@ -1727,7 +1727,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { ast::ItemKind::Struct(..) => { for attr in attr::filter_by_name(&i.attrs[..], "repr") { for item in attr.meta_item_list().unwrap_or_else(Vec::new) { - if item.check_name_symbol(symbols::simd) { + if item.check_name(symbols::simd) { gate_feature_post!(&self, repr_simd, attr.span, "SIMD types are experimental and possibly buggy"); } @@ -1738,7 +1738,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { ast::ItemKind::Enum(..) => { for attr in attr::filter_by_name(&i.attrs[..], "repr") { for item in attr.meta_item_list().unwrap_or_else(Vec::new) { - if item.check_name_symbol(symbols::align) { + if item.check_name(symbols::align) { gate_feature_post!(&self, repr_align_enum, attr.span, "`#[repr(align(x))]` on enums is experimental"); } @@ -2062,7 +2062,7 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute], // Process the edition umbrella feature-gates first, to ensure // `edition_enabled_features` is completed before it's queried. for attr in krate_attrs { - if !attr.check_name_symbol(symbols::feature) { + if !attr.check_name(symbols::feature) { continue } @@ -2107,7 +2107,7 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute], } for attr in krate_attrs { - if !attr.check_name_symbol(symbols::feature) { + if !attr.check_name(symbols::feature) { continue } @@ -2237,7 +2237,7 @@ fn maybe_stage_features(span_handler: &Handler, krate: &ast::Crate, }; if !allow_features { for attr in &krate.attrs { - if attr.check_name_symbol(symbols::feature) { + if attr.check_name(symbols::feature) { let release_channel = option_env!("CFG_RELEASE_CHANNEL").unwrap_or("(unknown)"); span_err!(span_handler, attr.span, E0554, "#![feature] may not be used on the {} release channel", diff --git a/src/libsyntax_ext/proc_macro_decls.rs b/src/libsyntax_ext/proc_macro_decls.rs index 22fee902aea26..f0390ba3d40cb 100644 --- a/src/libsyntax_ext/proc_macro_decls.rs +++ b/src/libsyntax_ext/proc_macro_decls.rs @@ -87,7 +87,7 @@ pub fn modify(sess: &ParseSess, } pub fn is_proc_macro_attr(attr: &ast::Attribute) -> bool { - PROC_MACRO_KINDS.iter().any(|kind| attr.check_name(kind)) + PROC_MACRO_KINDS.iter().any(|kind| attr.check_name(*kind)) } impl<'a> CollectProcMacros<'a> { diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index 0b00d4f9a54b1..f1f4c4d3598bc 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -100,7 +100,8 @@ symbols! { // Other symbols that can be referred to with syntax_pos::symbols::* Other { - doc, cfg, masked, spotlight, alias, keyword, feature, include, simd, align, + doc, cfg, masked, spotlight, alias, keyword, feature, include, simd, align, stable, + unstable, rustc_const_unstable, } } From 9c63475f22de85af443285c124af8244089589b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 9 Apr 2019 09:18:49 +0200 Subject: [PATCH 21/23] Ensure the symbols are pure strings --- src/libsyntax/ast.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 735b4ef4a3e52..ed207a1692a43 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -70,7 +70,13 @@ pub struct Path { impl PartialEq for Path { fn eq(&self, symbol: &Symbol) -> bool { - self.segments.len() == 1 && self.segments[0].ident.name.interned() == *symbol + self.segments.len() == 1 && { + let name = self.segments[0].ident.name; + // Make sure these symbols are pure strings + debug_assert!(!symbol.is_gensymed()); + debug_assert!(!name.is_gensymed()); + name == *symbol + } } } From 856c8a094607fc8fd6d0c95a428f08f8a43cbb27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 9 Apr 2019 09:36:17 +0200 Subject: [PATCH 22/23] Move modules outside the proc macro --- src/librustc_macros/src/symbols.rs | 42 ++++++++++++------------------ src/libsyntax_pos/symbol.rs | 28 ++++++++++++++++++++ 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/librustc_macros/src/symbols.rs b/src/librustc_macros/src/symbols.rs index e72ab7f84e92a..15f4d988a1f65 100644 --- a/src/librustc_macros/src/symbols.rs +++ b/src/librustc_macros/src/symbols.rs @@ -129,37 +129,27 @@ pub fn symbols(input: TokenStream) -> TokenStream { } TokenStream::from(quote! { - #[allow(non_upper_case_globals)] - pub mod keywords { - use super::{Symbol, Ident}; - #[derive(Clone, Copy, PartialEq, Eq)] - pub struct Keyword { - ident: Ident, - } - impl Keyword { - #[inline] pub fn ident(self) -> Ident { self.ident } - #[inline] pub fn name(self) -> Symbol { self.ident.name } - } - - #keyword_stream - - impl std::str::FromStr for Keyword { - type Err = (); - - fn from_str(s: &str) -> Result { - match s { - #from_str_stream - _ => Err(()), + macro_rules! keywords { + () => { + #keyword_stream + + impl std::str::FromStr for Keyword { + type Err = (); + + fn from_str(s: &str) -> Result { + match s { + #from_str_stream + _ => Err(()), + } } } } } - #[allow(non_upper_case_globals)] - pub mod symbols { - use super::Symbol; - - #symbols_stream + macro_rules! symbols { + () => { + #symbols_stream + } } impl Interner { diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index f1f4c4d3598bc..5b366d0a1e65c 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -406,6 +406,34 @@ impl Interner { } } +pub mod keywords { + use super::{Symbol, Ident}; + + #[derive(Clone, Copy, PartialEq, Eq)] + pub struct Keyword { + ident: Ident, + } + + impl Keyword { + #[inline] + pub fn ident(self) -> Ident { + self.ident + } + + #[inline] + pub fn name(self) -> Symbol { + self.ident.name + } + } + + keywords!(); +} + +pub mod symbols { + use super::Symbol; + symbols!(); +} + impl Symbol { fn is_used_keyword_2018(self) -> bool { self == keywords::Dyn.name() From b5f246aa70eb029a95b977fc51c5b74d66748a16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 9 Apr 2019 09:39:48 +0200 Subject: [PATCH 23/23] Use colon for keyword defs --- src/librustc_macros/src/symbols.rs | 2 +- src/libsyntax_pos/symbol.rs | 122 ++++++++++++++--------------- 2 files changed, 62 insertions(+), 62 deletions(-) diff --git a/src/librustc_macros/src/symbols.rs b/src/librustc_macros/src/symbols.rs index 15f4d988a1f65..169c5e1371833 100644 --- a/src/librustc_macros/src/symbols.rs +++ b/src/librustc_macros/src/symbols.rs @@ -22,7 +22,7 @@ struct Keyword { impl Parse for Keyword { fn parse(input: ParseStream<'_>) -> Result { let name = input.parse()?; - input.parse::()?; + input.parse::()?; let value = input.parse()?; input.parse::()?; diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index 5b366d0a1e65c..64dbb500d2e32 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -23,79 +23,79 @@ symbols! { Keywords { // Special reserved identifiers used internally for elided lifetimes, // unnamed method parameters, crate root module, error recovery etc. - Invalid, "", - PathRoot, "{{root}}", - DollarCrate, "$crate", - Underscore, "_", + Invalid: "", + PathRoot: "{{root}}", + DollarCrate: "$crate", + Underscore: "_", // Keywords that are used in stable Rust. - As, "as", - Box, "box", - Break, "break", - Const, "const", - Continue, "continue", - Crate, "crate", - Else, "else", - Enum, "enum", - Extern, "extern", - False, "false", - Fn, "fn", - For, "for", - If, "if", - Impl, "impl", - In, "in", - Let, "let", - Loop, "loop", - Match, "match", - Mod, "mod", - Move, "move", - Mut, "mut", - Pub, "pub", - Ref, "ref", - Return, "return", - SelfLower, "self", - SelfUpper, "Self", - Static, "static", - Struct, "struct", - Super, "super", - Trait, "trait", - True, "true", - Type, "type", - Unsafe, "unsafe", - Use, "use", - Where, "where", - While, "while", + As: "as", + Box: "box", + Break: "break", + Const: "const", + Continue: "continue", + Crate: "crate", + Else: "else", + Enum: "enum", + Extern: "extern", + False: "false", + Fn: "fn", + For: "for", + If: "if", + Impl: "impl", + In: "in", + Let: "let", + Loop: "loop", + Match: "match", + Mod: "mod", + Move: "move", + Mut: "mut", + Pub: "pub", + Ref: "ref", + Return: "return", + SelfLower: "self", + SelfUpper: "Self", + Static: "static", + Struct: "struct", + Super: "super", + Trait: "trait", + True: "true", + Type: "type", + Unsafe: "unsafe", + Use: "use", + Where: "where", + While: "while", // Keywords that are used in unstable Rust or reserved for future use. - Abstract, "abstract", - Become, "become", - Do, "do", - Final, "final", - Macro, "macro", - Override, "override", - Priv, "priv", - Typeof, "typeof", - Unsized, "unsized", - Virtual, "virtual", - Yield, "yield", + Abstract: "abstract", + Become: "become", + Do: "do", + Final: "final", + Macro: "macro", + Override: "override", + Priv: "priv", + Typeof: "typeof", + Unsized: "unsized", + Virtual: "virtual", + Yield: "yield", // Edition-specific keywords that are used in stable Rust. - Dyn, "dyn", // >= 2018 Edition only + Dyn: "dyn", // >= 2018 Edition only // Edition-specific keywords that are used in unstable Rust or reserved for future use. - Async, "async", // >= 2018 Edition only - Try, "try", // >= 2018 Edition only + Async: "async", // >= 2018 Edition only + Try: "try", // >= 2018 Edition only // Special lifetime names - UnderscoreLifetime, "'_", - StaticLifetime, "'static", + UnderscoreLifetime: "'_", + StaticLifetime: "'static", // Weak keywords, have special meaning only in specific contexts. - Auto, "auto", - Catch, "catch", - Default, "default", - Existential, "existential", - Union, "union", + Auto: "auto", + Catch: "catch", + Default: "default", + Existential: "existential", + Union: "union", } // Other symbols that can be referred to with syntax_pos::symbols::*