From 287edaa3b7be00af1a6224a5f0d87d47865f50d0 Mon Sep 17 00:00:00 2001 From: David Venhoek Date: Fri, 12 Aug 2022 13:54:55 +0200 Subject: [PATCH] This allows macro code to use identifiers for local variables which are also defined as globals outside of the macro, so long as their definition is not ambiguous. This improves predictability of code containing macros, and reduces the leakage of macro implementation details to their surroundings. Concretely, constructions such as ```rust thread_local!(static FOO: usize = 0); static init: usize = 0; ``` no longer generate unexpected error messages on the implementation detail that thread_local! defines a function with init as a parameter name. --- compiler/rustc_resolve/src/late.rs | 63 ++++++++++++------- .../ui/hygiene/shadow-macro-extern-allowed.rs | 26 ++++++++ .../shadow-macro-generated-forbidden.rs | 19 ++++++ .../shadow-macro-generated-forbidden.stderr | 21 +++++++ .../shadow-macro-internal-forbidden.rs | 25 ++++++++ .../shadow-macro-internal-forbidden.stderr | 44 +++++++++++++ 6 files changed, 176 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/hygiene/shadow-macro-extern-allowed.rs create mode 100644 src/test/ui/hygiene/shadow-macro-generated-forbidden.rs create mode 100644 src/test/ui/hygiene/shadow-macro-generated-forbidden.stderr create mode 100644 src/test/ui/hygiene/shadow-macro-internal-forbidden.rs create mode 100644 src/test/ui/hygiene/shadow-macro-internal-forbidden.stderr diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 58a4cff55db7d..596b5261729b0 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -3191,33 +3191,52 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // but we still conservatively report an error, see // issues/33118#issuecomment-233962221 for one reason why. let binding = binding.expect("no binding for a ctor or static"); - self.report_error( - ident.span, - ResolutionError::BindingShadowsSomethingUnacceptable { - shadowing_binding: pat_src, - name: ident.name, - participle: if binding.is_import() { "imported" } else { "defined" }, - article: binding.res().article(), - shadowed_binding: binding.res(), - shadowed_binding_span: binding.span, - }, - ); + // When working with macros we dont always want to report an error + // here, because doing so can result in the leaking of variable + // names and such out of a macro. At the same time, the confusion + // that can be caused by the variable definition hiding a constant + // in pattern matching is less of a concern in macros, as it + // probably doesn't want (or expect) the constant anyway. Allowing + // the shadow in these circumstances improves predictability and + // makes stability guarantees simpler. + // + // We need to do the context check asymetrically here to properly account + // for the fact that we only want to ignore constants comming into the macro + // not those defined inside it seen from a containing context. + let ident_context = ident.span.ctxt().normalize_to_macro_rules(); + if binding.span.ctxt().outer_expn().is_descendant_of(ident_context.outer_expn()) { + self.report_error( + ident.span, + ResolutionError::BindingShadowsSomethingUnacceptable { + shadowing_binding: pat_src, + name: ident.name, + participle: if binding.is_import() { "imported" } else { "defined" }, + article: binding.res().article(), + shadowed_binding: binding.res(), + shadowed_binding_span: binding.span, + }, + ); + } None } Res::Def(DefKind::ConstParam, def_id) => { // Same as for DefKind::Const above, but here, `binding` is `None`, so we // have to construct the error differently - self.report_error( - ident.span, - ResolutionError::BindingShadowsSomethingUnacceptable { - shadowing_binding: pat_src, - name: ident.name, - participle: "defined", - article: res.article(), - shadowed_binding: res, - shadowed_binding_span: self.r.opt_span(def_id).expect("const parameter defined outside of local crate"), - } - ); + let ident_context = ident.span.ctxt().normalize_to_macro_rules(); + let shadowed_binding_span = self.r.opt_span(def_id).expect("const parameter defined outside of local crate"); + if shadowed_binding_span.ctxt().outer_expn().is_descendant_of(ident_context.outer_expn()) { + self.report_error( + ident.span, + ResolutionError::BindingShadowsSomethingUnacceptable { + shadowing_binding: pat_src, + name: ident.name, + participle: "defined", + article: res.article(), + shadowed_binding: res, + shadowed_binding_span, + } + ); + } None } Res::Def(DefKind::Fn, _) | Res::Local(..) | Res::Err => { diff --git a/src/test/ui/hygiene/shadow-macro-extern-allowed.rs b/src/test/ui/hygiene/shadow-macro-extern-allowed.rs new file mode 100644 index 0000000000000..08f2e2e823601 --- /dev/null +++ b/src/test/ui/hygiene/shadow-macro-extern-allowed.rs @@ -0,0 +1,26 @@ +// build-pass + +// Check that a macro let binding that is unambiguous is allowed +// to shadow externally defined constants + +macro_rules! h { + () => { + let x @ _ = 2; + let y @ _ = 3; + } +} + +#[allow(non_upper_case_globals)] +const x: usize = 4; + +#[allow(non_upper_case_globals)] +const y: usize = 5; + +#[allow(non_upper_case_globals)] +fn a() { + h!(); +} + +fn main() { + h!(); +} diff --git a/src/test/ui/hygiene/shadow-macro-generated-forbidden.rs b/src/test/ui/hygiene/shadow-macro-generated-forbidden.rs new file mode 100644 index 0000000000000..0028a774896d5 --- /dev/null +++ b/src/test/ui/hygiene/shadow-macro-generated-forbidden.rs @@ -0,0 +1,19 @@ +// Check that macro-generated statics and consts are not allowed to be shadowed + +macro_rules! h { + () => { + #[allow(non_upper_case_globals)] + static x: usize = 2; + #[allow(non_upper_case_globals)] + const y: usize = 3; + } +} + +h!(); + +fn main() { + let x @ _ = 4; + //~^ ERROR let bindings cannot shadow statics + let y @ _ = 5; + //~^ ERROR let bindings cannot shadow constants +} diff --git a/src/test/ui/hygiene/shadow-macro-generated-forbidden.stderr b/src/test/ui/hygiene/shadow-macro-generated-forbidden.stderr new file mode 100644 index 0000000000000..80cd24490df02 --- /dev/null +++ b/src/test/ui/hygiene/shadow-macro-generated-forbidden.stderr @@ -0,0 +1,21 @@ +error[E0530]: let bindings cannot shadow statics + --> $DIR/shadow-macro-generated-forbidden.rs:15:9 + | +LL | static x: usize = 2; + | -------------------- the static `x` is defined here +... +LL | let x @ _ = 4; + | ^ cannot be named the same as a static + +error[E0530]: let bindings cannot shadow constants + --> $DIR/shadow-macro-generated-forbidden.rs:17:9 + | +LL | const y: usize = 3; + | ------------------- the constant `y` is defined here +... +LL | let y @ _ = 5; + | ^ cannot be named the same as a constant + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0530`. diff --git a/src/test/ui/hygiene/shadow-macro-internal-forbidden.rs b/src/test/ui/hygiene/shadow-macro-internal-forbidden.rs new file mode 100644 index 0000000000000..84af292b46316 --- /dev/null +++ b/src/test/ui/hygiene/shadow-macro-internal-forbidden.rs @@ -0,0 +1,25 @@ +// Test that shadowing of statics and constants from within the same macro is still forbidden. + +macro_rules! h { + () => { + #[allow(non_upper_case_globals)] + static x: usize = 2; + #[allow(non_upper_case_globals)] + const y: usize = 3; + fn a() { + let x @ _ = 4; + //~^ ERROR let bindings cannot shadow statics + let y @ _ = 5; + //~^ ERROR let bindings cannot shadow constants + } + #[allow(non_upper_case_globals)] + fn b() { + let z @ _ = 21; + //~^ ERROR let bindings cannot shadow const parameters + } + } +} + +h!(); + +fn main() {} diff --git a/src/test/ui/hygiene/shadow-macro-internal-forbidden.stderr b/src/test/ui/hygiene/shadow-macro-internal-forbidden.stderr new file mode 100644 index 0000000000000..1358b322f1369 --- /dev/null +++ b/src/test/ui/hygiene/shadow-macro-internal-forbidden.stderr @@ -0,0 +1,44 @@ +error[E0530]: let bindings cannot shadow statics + --> $DIR/shadow-macro-internal-forbidden.rs:10:17 + | +LL | static x: usize = 2; + | -------------------- the static `x` is defined here +... +LL | let x @ _ = 4; + | ^ cannot be named the same as a static +... +LL | h!(); + | ---- in this macro invocation + | + = note: this error originates in the macro `h` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0530]: let bindings cannot shadow constants + --> $DIR/shadow-macro-internal-forbidden.rs:12:17 + | +LL | const y: usize = 3; + | ------------------- the constant `y` is defined here +... +LL | let y @ _ = 5; + | ^ cannot be named the same as a constant +... +LL | h!(); + | ---- in this macro invocation + | + = note: this error originates in the macro `h` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0530]: let bindings cannot shadow const parameters + --> $DIR/shadow-macro-internal-forbidden.rs:17:17 + | +LL | fn b() { + | - the const parameter `z` is defined here +LL | let z @ _ = 21; + | ^ cannot be named the same as a const parameter +... +LL | h!(); + | ---- in this macro invocation + | + = note: this error originates in the macro `h` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0530`.