Skip to content

Loosen shadowing check inside macro contexts (attempt 2). #100453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 41 additions & 22 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
26 changes: 26 additions & 0 deletions src/test/ui/hygiene/shadow-macro-extern-allowed.rs
Original file line number Diff line number Diff line change
@@ -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<const x: usize>() {
h!();
}

fn main() {
h!();
Copy link
Contributor

@cjgillot cjgillot Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should this print?

macro_rules! h2 { ($z:ident) => {
  let x @ _ = 3;
  eprintln!("{} {}", x, $z);
}}

const x: u32 = 5;
fn main() {
  h2!(x)
}

}
19 changes: 19 additions & 0 deletions src/test/ui/hygiene/shadow-macro-generated-forbidden.rs
Original file line number Diff line number Diff line change
@@ -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
}
21 changes: 21 additions & 0 deletions src/test/ui/hygiene/shadow-macro-generated-forbidden.stderr
Original file line number Diff line number Diff line change
@@ -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`.
25 changes: 25 additions & 0 deletions src/test/ui/hygiene/shadow-macro-internal-forbidden.rs
Original file line number Diff line number Diff line change
@@ -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<const z: usize>() {
let z @ _ = 21;
//~^ ERROR let bindings cannot shadow const parameters
}
}
}

h!();

fn main() {}
44 changes: 44 additions & 0 deletions src/test/ui/hygiene/shadow-macro-internal-forbidden.stderr
Original file line number Diff line number Diff line change
@@ -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<const z: usize>() {
| - 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`.