From 1deef26324a91fdf09e586a8982b0d4ce45ec864 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Wed, 30 Jun 2021 16:42:54 +0200 Subject: [PATCH 1/8] Improve wording of the `drop_bounds` lint --- compiler/rustc_lint/src/traits.rs | 36 +++++++++++++++++-------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_lint/src/traits.rs b/compiler/rustc_lint/src/traits.rs index e632f29e672c0..1c9f23932d17e 100644 --- a/compiler/rustc_lint/src/traits.rs +++ b/compiler/rustc_lint/src/traits.rs @@ -18,23 +18,27 @@ declare_lint! { /// /// ### Explanation /// - /// `Drop` bounds do not really accomplish anything. A type may have - /// compiler-generated drop glue without implementing the `Drop` trait - /// itself. The `Drop` trait also only has one method, `Drop::drop`, and - /// that function is by fiat not callable in user code. So there is really - /// no use case for using `Drop` in trait bounds. + /// A generic trait bound of the form `T: Drop` is most likely misleading + /// and not what the programmer intended (they probably should have used + /// `std::mem::needs_drop` instead). /// - /// The most likely use case of a drop bound is to distinguish between - /// types that have destructors and types that don't. Combined with - /// specialization, a naive coder would write an implementation that - /// assumed a type could be trivially dropped, then write a specialization - /// for `T: Drop` that actually calls the destructor. Except that doing so - /// is not correct; String, for example, doesn't actually implement Drop, - /// but because String contains a Vec, assuming it can be trivially dropped - /// will leak memory. + /// `Drop` bounds do not actually indicate whether a type can be trivially + /// dropped or not, because a composite type containing `Drop` types does + /// not necessarily implement `Drop` itself. Naïvely, one might be tempted + /// to write an implementation that assumes that a type can be trivially + /// dropped while also supplying a specialization for `T: Drop` that + /// actually calls the destructor. However, this breaks down e.g. when `T` + /// is `String`, which does not implement `Drop` itself but contains a + /// `Vec`, which does implement `Drop`, so assuming `T` can be trivially + /// dropped would lead to a memory leak here. + /// + /// Furthermore, the `Drop` trait only contains one method, `Drop::drop`, + /// which may not be called explicitly in user code (`E0040`), so there is + /// really no use case for using `Drop` in trait bounds, save perhaps for + /// some obscure corner cases, which can use `#[allow(drop_bounds)]`. pub DROP_BOUNDS, Warn, - "bounds of the form `T: Drop` are useless" + "bounds of the form `T: Drop` are most likely incorrect" } declare_lint_pass!( @@ -65,8 +69,8 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints { None => return, }; let msg = format!( - "bounds on `{}` are useless, consider instead \ - using `{}` to detect if a type has a destructor", + "bounds on `{}` are most likely incorrect, consider instead \ + using `{}` to detect whether a type can be trivially dropped", predicate, cx.tcx.def_path_str(needs_drop) ); From 644529bdf133ce0f86dc93b4942f9d16960c84ea Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Wed, 30 Jun 2021 18:06:11 +0200 Subject: [PATCH 2/8] Bless `src/test/ui/drop-bounds/drop-bounds.rs` --- src/test/ui/drop-bounds/drop-bounds.stderr | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/ui/drop-bounds/drop-bounds.stderr b/src/test/ui/drop-bounds/drop-bounds.stderr index 15ba4c9a64989..3ffb855a55dc8 100644 --- a/src/test/ui/drop-bounds/drop-bounds.stderr +++ b/src/test/ui/drop-bounds/drop-bounds.stderr @@ -1,4 +1,4 @@ -error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `T: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:2:11 | LL | fn foo() {} @@ -10,37 +10,37 @@ note: the lint level is defined here LL | #![deny(drop_bounds)] | ^^^^^^^^^^^ -error: bounds on `U: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `U: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:5:8 | LL | U: Drop, | ^^^^ -error: bounds on `impl Drop: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `impl Drop: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:8:17 | LL | fn baz(_x: impl Drop) {} | ^^^^ -error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `T: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:9:15 | LL | struct Foo { | ^^^^ -error: bounds on `U: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `U: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:12:24 | LL | struct Bar where U: Drop { | ^^^^ -error: bounds on `Self: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `Self: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:15:12 | LL | trait Baz: Drop { | ^^^^ -error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `T: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:17:9 | LL | impl Baz for T { From 4c0ff4db80d2c6cb4dc204f65f34ce2246e27bfe Mon Sep 17 00:00:00 2001 From: Maarten de Vries Date: Thu, 15 Jul 2021 21:25:11 +0200 Subject: [PATCH 3/8] Show discriminant before overflow in diagnostic. --- compiler/rustc_typeck/src/check/check.rs | 27 ++++++++++++++++--- src/test/ui/enum/enum-discrim-autosizing.rs | 6 +++-- .../ui/enum/enum-discrim-autosizing.stderr | 2 +- src/test/ui/error-codes/E0081.rs | 11 ++++++++ src/test/ui/error-codes/E0081.stderr | 14 ++++++++-- 5 files changed, 52 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index b5db3331d0447..9b555df4b92de 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -1472,15 +1472,17 @@ fn check_enum<'tcx>( Some(ref expr) => tcx.hir().span(expr.hir_id), None => v.span, }; + let display_discr = display_discriminant_value(tcx, v, discr.val); + let display_discr_i = display_discriminant_value(tcx, variant_i, disr_vals[i].val); struct_span_err!( tcx.sess, span, E0081, "discriminant value `{}` already exists", - disr_vals[i] + discr.val, ) - .span_label(i_span, format!("first use of `{}`", disr_vals[i])) - .span_label(span, format!("enum already has `{}`", disr_vals[i])) + .span_label(i_span, format!("first use of {}", display_discr_i)) + .span_label(span, format!("enum already has {}", display_discr)) .emit(); } disr_vals.push(discr); @@ -1490,6 +1492,25 @@ fn check_enum<'tcx>( check_transparent(tcx, sp, def); } +/// Format an enum discriminant value for use in a diagnostic message. +fn display_discriminant_value<'tcx>( + tcx: TyCtxt<'tcx>, + variant: &hir::Variant<'_>, + evaluated: u128, +) -> String { + if let Some(expr) = &variant.disr_expr { + let body = &tcx.hir().body(expr.body).value; + if let hir::ExprKind::Lit(lit) = &body.kind { + if let rustc_ast::LitKind::Int(lit_value, _int_kind) = &lit.node { + if evaluated != *lit_value { + return format!("`{}` (overflowed from `{}`)", evaluated, lit_value); + } + } + } + } + format!("`{}`", evaluated) +} + pub(super) fn check_type_params_are_used<'tcx>( tcx: TyCtxt<'tcx>, generics: &ty::Generics, diff --git a/src/test/ui/enum/enum-discrim-autosizing.rs b/src/test/ui/enum/enum-discrim-autosizing.rs index e9bb927597100..473a0ad6f7a13 100644 --- a/src/test/ui/enum/enum-discrim-autosizing.rs +++ b/src/test/ui/enum/enum-discrim-autosizing.rs @@ -4,8 +4,10 @@ // so force the repr. #[cfg_attr(not(target_pointer_width = "32"), repr(i32))] enum Eu64 { - Au64 = 0, - Bu64 = 0x8000_0000_0000_0000 //~ERROR already exists + Au64 = 0, //~NOTE first use of `0` + Bu64 = 0x8000_0000_0000_0000 + //~^ ERROR discriminant value `0` already exists + //~| NOTE enum already has `0` (overflowed from `9223372036854775808`) } fn main() {} diff --git a/src/test/ui/enum/enum-discrim-autosizing.stderr b/src/test/ui/enum/enum-discrim-autosizing.stderr index 8848f984cfb7d..a0f39098a2e98 100644 --- a/src/test/ui/enum/enum-discrim-autosizing.stderr +++ b/src/test/ui/enum/enum-discrim-autosizing.stderr @@ -4,7 +4,7 @@ error[E0081]: discriminant value `0` already exists LL | Au64 = 0, | - first use of `0` LL | Bu64 = 0x8000_0000_0000_0000 - | ^^^^^^^^^^^^^^^^^^^^^ enum already has `0` + | ^^^^^^^^^^^^^^^^^^^^^ enum already has `0` (overflowed from `9223372036854775808`) error: aborting due to previous error diff --git a/src/test/ui/error-codes/E0081.rs b/src/test/ui/error-codes/E0081.rs index 33c8c14306bd1..255e05ced19f7 100644 --- a/src/test/ui/error-codes/E0081.rs +++ b/src/test/ui/error-codes/E0081.rs @@ -1,9 +1,20 @@ enum Enum { P = 3, + //~^ NOTE first use of `3` X = 3, //~^ ERROR discriminant value `3` already exists + //~| NOTE enum already has `3` Y = 5 } +#[repr(u8)] +enum EnumOverflowRepr { + P = 257, + //~^ NOTE first use of `1` (overflowed from `257`) + X = 513, + //~^ ERROR discriminant value `1` already exists + //~| NOTE enum already has `1` (overflowed from `513`) +} + fn main() { } diff --git a/src/test/ui/error-codes/E0081.stderr b/src/test/ui/error-codes/E0081.stderr index 0b3d351e1ac9a..9b279bb0214c6 100644 --- a/src/test/ui/error-codes/E0081.stderr +++ b/src/test/ui/error-codes/E0081.stderr @@ -1,11 +1,21 @@ error[E0081]: discriminant value `3` already exists - --> $DIR/E0081.rs:3:9 + --> $DIR/E0081.rs:4:9 | LL | P = 3, | - first use of `3` +LL | LL | X = 3, | ^ enum already has `3` -error: aborting due to previous error +error[E0081]: discriminant value `1` already exists + --> $DIR/E0081.rs:14:9 + | +LL | P = 257, + | --- first use of `1` (overflowed from `257`) +LL | +LL | X = 513, + | ^^^ enum already has `1` (overflowed from `513`) + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0081`. From 13e2f807a19398e735ca3addab19366cda133a31 Mon Sep 17 00:00:00 2001 From: kit Date: Mon, 16 Aug 2021 15:35:53 +1000 Subject: [PATCH 4/8] Generate an iOS LLVM target with a specific version Without the specific version, the mach-o header will be missing the minimum supported operating system version. This is mandatory for running Rust binaries on iOS devices. --- compiler/rustc_target/src/spec/aarch64_apple_ios.rs | 9 ++++++++- compiler/rustc_target/src/spec/apple_base.rs | 5 +++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/spec/aarch64_apple_ios.rs b/compiler/rustc_target/src/spec/aarch64_apple_ios.rs index e5805d9e691cb..6468419fce7c3 100644 --- a/compiler/rustc_target/src/spec/aarch64_apple_ios.rs +++ b/compiler/rustc_target/src/spec/aarch64_apple_ios.rs @@ -2,8 +2,15 @@ use super::apple_sdk_base::{opts, Arch}; use crate::spec::{FramePointer, Target, TargetOptions}; pub fn target() -> Target { + // Clang automatically chooses a more specific target based on + // IPHONEOS_DEPLOYMENT_TARGET. + // This is required for the target to pick the right + // MACH-O commands, so we do too. + let arch = "arm64"; + let llvm_target = super::apple_base::ios_llvm_target(arch); + Target { - llvm_target: "arm64-apple-ios".to_string(), + llvm_target, pointer_width: 64, data_layout: "e-m:o-i64:64-i128:128-n32:64-S128".to_string(), arch: "aarch64".to_string(), diff --git a/compiler/rustc_target/src/spec/apple_base.rs b/compiler/rustc_target/src/spec/apple_base.rs index 0c8a89210ffcd..cff0b3651e170 100644 --- a/compiler/rustc_target/src/spec/apple_base.rs +++ b/compiler/rustc_target/src/spec/apple_base.rs @@ -91,6 +91,11 @@ fn ios_deployment_target() -> (u32, u32) { deployment_target("IPHONEOS_DEPLOYMENT_TARGET").unwrap_or((7, 0)) } +pub fn ios_llvm_target(arch: &str) -> String { + let (major, minor) = ios_deployment_target(); + format!("{}-apple-ios{}.{}.0", arch, major, minor) +} + pub fn ios_sim_llvm_target(arch: &str) -> String { let (major, minor) = ios_deployment_target(); format!("{}-apple-ios{}.{}.0-simulator", arch, major, minor) From c4e6185385148ec964d2765957a83bdf3df7b112 Mon Sep 17 00:00:00 2001 From: Augie Fackler Date: Thu, 19 Aug 2021 12:40:37 -0400 Subject: [PATCH 5/8] PassWrapper: adapt for LLVM 14 changes These API changes appear to have all taken place in https://reviews.llvm.org/D105007, which moved HWAddressSanitizerPass and AddressSanitizerPass to only accept their options type as a ctor argument instead of the sequence of bools etc. This required a couple of parameter additions, which I made match the default prior to the mentioned upstream LLVM change. This patch restores rustc to building (though not quite passing all tests, I've mailed other patches for those issues) against LLVM HEAD. --- compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp index f563870e3e0c7..b3f86f3295ae9 100644 --- a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp @@ -922,9 +922,17 @@ LLVMRustOptimizeWithNewPassManager( MPM.addPass(RequireAnalysisPass()); MPM.addPass(ModuleAddressSanitizerPass( /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover)); +#if LLVM_VERSION_GE(14, 0) + AddressSanitizerOptions opts(/*CompileKernel=*/false, + SanitizerOptions->SanitizeAddressRecover, + /*UseAfterScope=*/true, + AsanDetectStackUseAfterReturnMode::Runtime); + MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass(opts))); +#else MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass( /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover, /*UseAfterScope=*/true))); +#endif } ); #else @@ -952,8 +960,15 @@ LLVMRustOptimizeWithNewPassManager( #if LLVM_VERSION_GE(11, 0) OptimizerLastEPCallbacks.push_back( [SanitizerOptions](ModulePassManager &MPM, OptimizationLevel Level) { +#if LLVM_VERSION_GE(14, 0) + HWAddressSanitizerOptions opts( + /*CompileKernel=*/false, SanitizerOptions->SanitizeHWAddressRecover, + /*DisableOptimization=*/false); + MPM.addPass(HWAddressSanitizerPass(opts)); +#else MPM.addPass(HWAddressSanitizerPass( /*CompileKernel=*/false, SanitizerOptions->SanitizeHWAddressRecover)); +#endif } ); #else From 1df0b73196d0830413cc37659fe9da3a52002bd6 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 21 Aug 2021 18:07:21 +0300 Subject: [PATCH 6/8] cleanup: `Span::new` -> `Span::with_lo` --- compiler/rustc_middle/src/middle/region.rs | 2 +- compiler/rustc_typeck/src/check/compare_method.rs | 12 ++---------- .../clippy_lints/src/unit_return_expecting_ord.rs | 2 +- src/tools/clippy/clippy_lints/src/write.rs | 2 +- src/tools/clippy/clippy_utils/src/lib.rs | 2 +- 5 files changed, 6 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_middle/src/middle/region.rs b/compiler/rustc_middle/src/middle/region.rs index f44267a404bf3..ffa26b9f299cb 100644 --- a/compiler/rustc_middle/src/middle/region.rs +++ b/compiler/rustc_middle/src/middle/region.rs @@ -189,7 +189,7 @@ impl Scope { // To avoid issues with macro-generated spans, the span // of the statement must be nested in that of the block. if span.lo() <= stmt_span.lo() && stmt_span.lo() <= span.hi() { - return Span::new(stmt_span.lo(), span.hi(), span.ctxt()); + return span.with_lo(stmt_span.lo()); } } } diff --git a/compiler/rustc_typeck/src/check/compare_method.rs b/compiler/rustc_typeck/src/check/compare_method.rs index 316a097556aec..8d5bf98be9932 100644 --- a/compiler/rustc_typeck/src/check/compare_method.rs +++ b/compiler/rustc_typeck/src/check/compare_method.rs @@ -708,11 +708,7 @@ fn compare_number_of_method_arguments<'tcx>( Some(if pos == 0 { arg.span } else { - Span::new( - trait_m_sig.decl.inputs[0].span.lo(), - arg.span.hi(), - arg.span.ctxt(), - ) + arg.span.with_lo(trait_m_sig.decl.inputs[0].span.lo()) }) } else { trait_item_span @@ -731,11 +727,7 @@ fn compare_number_of_method_arguments<'tcx>( if pos == 0 { arg.span } else { - Span::new( - impl_m_sig.decl.inputs[0].span.lo(), - arg.span.hi(), - arg.span.ctxt(), - ) + arg.span.with_lo(impl_m_sig.decl.inputs[0].span.lo()) } } else { impl_m_span diff --git a/src/tools/clippy/clippy_lints/src/unit_return_expecting_ord.rs b/src/tools/clippy/clippy_lints/src/unit_return_expecting_ord.rs index ee675838c4cb3..db0f412f2a18c 100644 --- a/src/tools/clippy/clippy_lints/src/unit_return_expecting_ord.rs +++ b/src/tools/clippy/clippy_lints/src/unit_return_expecting_ord.rs @@ -127,7 +127,7 @@ fn check_arg<'tcx>(cx: &LateContext<'tcx>, arg: &'tcx Expr<'tcx>) -> Option<(Spa then { let data = stmt.span.data(); // Make a span out of the semicolon for the help message - Some((span, Some(Span::new(data.hi-BytePos(1), data.hi, data.ctxt)))) + Some((span, Some(data.with_lo(data.hi-BytePos(1))))) } else { Some((span, None)) } diff --git a/src/tools/clippy/clippy_lints/src/write.rs b/src/tools/clippy/clippy_lints/src/write.rs index 4553ac704a28f..85d1f65c51f09 100644 --- a/src/tools/clippy/clippy_lints/src/write.rs +++ b/src/tools/clippy/clippy_lints/src/write.rs @@ -299,7 +299,7 @@ impl EarlyLintPass for Write { let nl_span = match (dest, only_nl) { // Special case of `write!(buf, "\n")`: Mark everything from the end of // `buf` for removal so no trailing comma [`writeln!(buf, )`] remains. - (Some(dest_expr), true) => Span::new(dest_expr.span.hi(), nl_span.hi(), nl_span.ctxt()), + (Some(dest_expr), true) => nl_span.with_lo(dest_expr.span.hi()), _ => nl_span, }; span_lint_and_then( diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 82bfce8fe789e..0da86a76d39eb 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -877,7 +877,7 @@ fn line_span(cx: &T, span: Span) -> Span { let source_map_and_line = cx.sess().source_map().lookup_line(span.lo()).unwrap(); let line_no = source_map_and_line.line; let line_start = source_map_and_line.sf.lines[line_no]; - Span::new(line_start, span.hi(), span.ctxt()) + span.with_lo(line_start) } /// Gets the parent node, if any. From bcc5ecb969579eb1c5fd4df0f7855105b66fe907 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 22 Aug 2021 14:06:45 +0200 Subject: [PATCH 7/8] Suggest importing the right kind of macro. --- compiler/rustc_resolve/src/diagnostics.rs | 4 +-- src/test/ui/macros/issue-88228.rs | 22 ++++++++++++++++ src/test/ui/macros/issue-88228.stderr | 26 +++++++++++++++++++ .../proc-macro/derive-helper-shadowing.stderr | 4 +++ 4 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/macros/issue-88228.rs create mode 100644 src/test/ui/macros/issue-88228.stderr diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 0a5da653fab07..3cf042687562b 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -950,9 +950,7 @@ impl<'a> Resolver<'a> { self.add_typo_suggestion(err, suggestion, ident.span); let import_suggestions = - self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, |res| { - matches!(res, Res::Def(DefKind::Macro(MacroKind::Bang), _)) - }); + self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected); show_candidates(err, None, &import_suggestions, false, true); if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) { diff --git a/src/test/ui/macros/issue-88228.rs b/src/test/ui/macros/issue-88228.rs new file mode 100644 index 0000000000000..615b865e9f1e1 --- /dev/null +++ b/src/test/ui/macros/issue-88228.rs @@ -0,0 +1,22 @@ +// compile-flags: -Z deduplicate-diagnostics=yes +// edition:2018 + +mod hey { + pub use Copy as Bla; + pub use std::println as bla; +} + +#[derive(Bla)] +//~^ ERROR cannot find derive macro `Bla` +//~| NOTE consider importing this derive macro +struct A; + +#[derive(println)] +//~^ ERROR cannot find derive macro `println` +struct B; + +fn main() { + bla!(); + //~^ ERROR cannot find macro `bla` + //~| NOTE consider importing this macro +} diff --git a/src/test/ui/macros/issue-88228.stderr b/src/test/ui/macros/issue-88228.stderr new file mode 100644 index 0000000000000..b164e39064c97 --- /dev/null +++ b/src/test/ui/macros/issue-88228.stderr @@ -0,0 +1,26 @@ +error: cannot find macro `bla` in this scope + --> $DIR/issue-88228.rs:19:5 + | +LL | bla!(); + | ^^^ + | + = note: consider importing this macro: + crate::hey::bla + +error: cannot find derive macro `println` in this scope + --> $DIR/issue-88228.rs:14:10 + | +LL | #[derive(println)] + | ^^^^^^^ + +error: cannot find derive macro `Bla` in this scope + --> $DIR/issue-88228.rs:9:10 + | +LL | #[derive(Bla)] + | ^^^ + | + = note: consider importing this derive macro: + crate::hey::Bla + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/proc-macro/derive-helper-shadowing.stderr b/src/test/ui/proc-macro/derive-helper-shadowing.stderr index 4115fec86fbc8..3b160935a2f8d 100644 --- a/src/test/ui/proc-macro/derive-helper-shadowing.stderr +++ b/src/test/ui/proc-macro/derive-helper-shadowing.stderr @@ -16,6 +16,8 @@ error: cannot find attribute `empty_helper` in this scope LL | #[derive(GenHelperUse)] | ^^^^^^^^^^^^ | + = note: consider importing this attribute macro: + empty_helper = note: this error originates in the derive macro `GenHelperUse` (in Nightly builds, run with -Z macro-backtrace for more info) error: cannot find attribute `empty_helper` in this scope @@ -27,6 +29,8 @@ LL | #[empty_helper] LL | gen_helper_use!(); | ------------------ in this macro invocation | + = note: consider importing this attribute macro: + crate::empty_helper = note: this error originates in the macro `gen_helper_use` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0659]: `empty_helper` is ambiguous (name vs any other name during import resolution) From abab99e02b3826355645889d6b53b33414e5f4d5 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 22 Aug 2021 16:50:59 +0200 Subject: [PATCH 8/8] Stop tracking namespce in used_imports. The information was tracked, but unused. --- compiler/rustc_resolve/src/check_unused.rs | 3 +-- compiler/rustc_resolve/src/imports.rs | 11 +++++------ compiler/rustc_resolve/src/late.rs | 4 ++-- compiler/rustc_resolve/src/lib.rs | 11 +++++------ compiler/rustc_resolve/src/macros.rs | 2 +- 5 files changed, 14 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index 9f4f2b82f6085..760b746996196 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -63,8 +63,7 @@ impl<'a, 'b> UnusedImportCheckVisitor<'a, 'b> { // We have information about whether `use` (import) items are actually // used now. If an import is not used at all, we signal a lint error. fn check_import(&mut self, id: ast::NodeId) { - let mut used = false; - self.r.per_ns(|this, ns| used |= this.used_imports.contains(&(id, ns))); + let used = self.r.used_imports.contains(&id); let def_id = self.r.local_def_id(id); if !used { if self.r.maybe_unused_trait_imports.contains(&def_id) { diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index acfa389fed58a..dfb6d89a0d126 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -303,7 +303,7 @@ impl<'a> Resolver<'a> { if self.last_import_segment && check_usable(self, binding).is_err() { Err((Determined, Weak::No)) } else { - self.record_use(ident, ns, binding, restricted_shadowing); + self.record_use(ident, binding, restricted_shadowing); if let Some(shadowed_glob) = resolution.shadowed_glob { // Forbid expanded shadowing to avoid time travel. @@ -609,9 +609,9 @@ impl<'a> Resolver<'a> { self.per_ns(|this, ns| { let key = this.new_key(target, ns); let _ = this.try_define(import.parent_scope.module, key, dummy_binding); - // Consider erroneous imports used to avoid duplicate diagnostics. - this.record_use(target, ns, dummy_binding, false); }); + // Consider erroneous imports used to avoid duplicate diagnostics. + self.record_use(target, dummy_binding, false); } } } @@ -709,7 +709,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { } } else if is_indeterminate { // Consider erroneous imports used to avoid duplicate diagnostics. - self.r.used_imports.insert((import.id, TypeNS)); + self.r.used_imports.insert(import.id); let path = import_path_to_string( &import.module_path.iter().map(|seg| seg.ident).collect::>(), &import.kind, @@ -902,7 +902,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { import.vis.set(orig_vis); if let PathResult::Failed { .. } | PathResult::NonModule(..) = path_res { // Consider erroneous imports used to avoid duplicate diagnostics. - self.r.used_imports.insert((import.id, TypeNS)); + self.r.used_imports.insert(import.id); } let module = match path_res { PathResult::Module(module) => { @@ -1043,7 +1043,6 @@ impl<'a, 'b> ImportResolver<'a, 'b> { { this.record_use( ident, - ns, target_binding, import.module_path.is_empty(), ); diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 00d291946df68..0b552aa07f517 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1738,7 +1738,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // whether they can be shadowed by fresh bindings or not, so force an error. // issues/33118#issuecomment-233962221 (see below) still applies here, // but we have to ignore it for backward compatibility. - self.r.record_use(ident, ValueNS, binding, false); + self.r.record_use(ident, binding, false); return None; } LexicalScopeBinding::Item(binding) => (binding.res(), Some(binding)), @@ -1753,7 +1753,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { ) if is_syntactic_ambiguity => { // Disambiguate in favor of a unit struct/variant or constant pattern. if let Some(binding) = binding { - self.r.record_use(ident, ValueNS, binding, false); + self.r.record_use(ident, binding, false); } Some(res) } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 7114fd33188d9..465007507dafe 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -942,7 +942,7 @@ pub struct Resolver<'a> { glob_map: FxHashMap>, /// Visibilities in "lowered" form, for all entities that have them. visibilities: FxHashMap, - used_imports: FxHashSet<(NodeId, Namespace)>, + used_imports: FxHashSet, maybe_unused_trait_imports: FxHashSet, maybe_unused_extern_crates: Vec<(LocalDefId, Span)>, @@ -1656,7 +1656,6 @@ impl<'a> Resolver<'a> { fn record_use( &mut self, ident: Ident, - ns: Namespace, used_binding: &'a NameBinding<'a>, is_lexical_scope: bool, ) { @@ -1684,9 +1683,9 @@ impl<'a> Resolver<'a> { } used.set(true); import.used.set(true); - self.used_imports.insert((import.id, ns)); + self.used_imports.insert(import.id); self.add_to_glob_map(&import, ident); - self.record_use(ident, ns, binding, false); + self.record_use(ident, binding, false); } } @@ -3241,7 +3240,7 @@ impl<'a> Resolver<'a> { self.extern_prelude.get(&ident.normalize_to_macros_2_0()).cloned().and_then(|entry| { if let Some(binding) = entry.extern_crate_item { if !speculative && entry.introduced_by_item { - self.record_use(ident, TypeNS, binding, false); + self.record_use(ident, binding, false); } Some(binding) } else { @@ -3428,7 +3427,7 @@ impl<'a> Resolver<'a> { let is_import = name_binding.is_import(); let span = name_binding.span; if let Res::Def(DefKind::Fn, _) = res { - self.record_use(ident, ValueNS, name_binding, false); + self.record_use(ident, name_binding, false); } self.main_def = Some(MainDefinition { res, is_import, span }); } diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index b2a8aa0ceccaa..7f86f891c4450 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -1090,7 +1090,7 @@ impl<'a> Resolver<'a> { ) { Ok(binding) => { let initial_res = initial_binding.map(|initial_binding| { - self.record_use(ident, MacroNS, initial_binding, false); + self.record_use(ident, initial_binding, false); initial_binding.res() }); let res = binding.res();