Skip to content

Commit

Permalink
Auto merge of #54069 - petrochenkov:subns, r=aturon
Browse files Browse the repository at this point in the history
resolve: Introduce two sub-namespaces in macro namespace

Two sub-namespaces are introduced in the macro namespace - one for bang macros and one for attribute-like macros (attributes, derives).

"Sub-namespace" means this is not a newly introduced full namespace, the single macro namespace is still in place.
I.e. you still can't define/import two macros with the same name in a single module, `use` imports still import only one name in macro namespace (from any sub-namespace) and not possibly two.

However, when we are searching for a name used in a `!` macro call context (`my_macro!()`) we skip attribute names in scope, and when we are searching for a name used in attribute context (`#[my_macro]`/`#[derive(my_macro)]`) we are skipping bang macro names in scope.
In other words, bang macros cannot shadow attribute macros and vice versa.

For a non-macro analogy, we could e.g. skip non-traits when searching for `MyTrait` in `impl MyTrait for Type { ... }`.
However we do not do it in non-macro namespaces because we don't have practical issues with e.g. non-traits shadowing traits with the same name, but with macros we do, especially after macro modularization.

For `#[test]` and `#[bench]` we have a hack in the compiler right now preventing their shadowing by `macro_rules! test` and similar things. This hack was introduced after making `#[test]`/`#[bench]` built-in macros instead of built-in attributes (#53410), something that needed to be done from the start since they are "active" attributes transforming their inputs.
Now they are passed through normal name resolution and can be shadowed, but that's a breaking change, so we have  a special hack basically applying this PR for `#[test]` and `#[bench]` only.

Soon all potentially built-in attributes will be passed through normal name resolution (#53913) and that uncovers even more cases where the strict "macro namespace is a single namespace" rule needs to be broken.
For example, with strict rules, built-in macro `cfg!(...)` would shadow built-in attribute `#[cfg]` (they are different things), standard library macro `thread_local!(...)` would shadow built-in attribute `#[thread_local]` - both of these cases are covered by special hacks in #53913 as well.
Crater run uncovered more cases of attributes being shadowed by user-defined macros (`warn`, `doc`, `main`, even `deprecated`), we cannot add exceptions in the compiler for all of them.

Regressions with user-defined attributes like #53583 and #53898 also appeared after enabling macro modularization.

People are also usually confused (#53205 (comment), #53583 (comment)) when they see conflicts between attributes and non-attribute macros for the first time.

So my proposed solution is to solve this issue by introducing two sub-namespaces and thus skipping resolutions of the wrong kind and preventing more error-causing cases of shadowing.

Fixes #53583
  • Loading branch information
bors committed Sep 14, 2018
2 parents 2ab3eba + beb3b5d commit f789b6b
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 35 deletions.
29 changes: 18 additions & 11 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,21 @@ pub struct ProcMacError {
warn_msg: &'static str,
}

// For compatibility bang macros are skipped when resolving potentially built-in attributes.
fn macro_kind_mismatch(name: Name, requirement: Option<MacroKind>, candidate: Option<MacroKind>)
-> bool {
requirement == Some(MacroKind::Attr) && candidate == Some(MacroKind::Bang) &&
(name == "test" || name == "bench" || is_builtin_attr_name(name))
// Macro namespace is separated into two sub-namespaces, one for bang macros and
// one for attribute-like macros (attributes, derives).
// We ignore resolutions from one sub-namespace when searching names in scope for another.
fn sub_namespace_mismatch(requirement: Option<MacroKind>, candidate: Option<MacroKind>) -> bool {
#[derive(PartialEq)]
enum SubNS { Bang, AttrLike }
let sub_ns = |kind| match kind {
MacroKind::Bang => Some(SubNS::Bang),
MacroKind::Attr | MacroKind::Derive => Some(SubNS::AttrLike),
MacroKind::ProcMacroStub => None,
};
let requirement = requirement.and_then(|kind| sub_ns(kind));
let candidate = candidate.and_then(|kind| sub_ns(kind));
// "No specific sub-namespace" means "matches anything" for both requirements and candidates.
candidate.is_some() && requirement.is_some() && candidate != requirement
}

impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
Expand Down Expand Up @@ -641,10 +651,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
}
}
WhereToResolve::BuiltinAttrs => {
// FIXME: Only built-in attributes are not considered as candidates for
// non-attributes to fight off regressions on stable channel (#53205).
// We need to come up with some more principled approach instead.
if kind == Some(MacroKind::Attr) && is_builtin_attr_name(ident.name) {
if is_builtin_attr_name(ident.name) {
let binding = (Def::NonMacroAttr(NonMacroAttrKind::Builtin),
ty::Visibility::Public, ident.span, Mark::root())
.to_name_binding(self.arenas);
Expand Down Expand Up @@ -765,7 +772,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {

match result {
Ok(result) => {
if macro_kind_mismatch(ident.name, kind, result.0.macro_kind()) {
if sub_namespace_mismatch(kind, result.0.macro_kind()) {
continue_search!();
}

Expand Down Expand Up @@ -829,7 +836,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
parent_scope: &ParentScope<'a>,
record_used: bool,
) -> Option<&'a NameBinding<'a>> {
if macro_kind_mismatch(ident.name, kind, Some(MacroKind::Bang)) {
if sub_namespace_mismatch(kind, Some(MacroKind::Bang)) {
return None;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#[macro_export]
macro_rules! my_attr { () => () }
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;
use proc_macro::*;

#[proc_macro_derive(MyTrait, attributes(my_attr))]
pub fn foo(_: TokenStream) -> TokenStream {
TokenStream::new()
}
16 changes: 16 additions & 0 deletions src/test/ui-fulldeps/proc-macro/derive-helper-shadowed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// compile-pass
// aux-build:derive-helper-shadowed.rs
// aux-build:derive-helper-shadowed-2.rs

#[macro_use]
extern crate derive_helper_shadowed;
#[macro_use(my_attr)]
extern crate derive_helper_shadowed_2;

macro_rules! my_attr { () => () }

#[derive(MyTrait)]
#[my_attr] // OK
struct S;

fn main() {}
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-11692-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
// except according to those terms.

fn main() {
concat!(test!()); //~ ERROR `test` can only be used in attributes
concat!(test!()); //~ ERROR cannot find macro `test!` in this scope
}
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-11692-2.stderr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
error: `test` can only be used in attributes
error: cannot find macro `test!` in this scope
--> $DIR/issue-11692-2.rs:12:13
|
LL | concat!(test!()); //~ ERROR `test` can only be used in attributes
LL | concat!(test!()); //~ ERROR cannot find macro `test!` in this scope
| ^^^^

error: aborting due to previous error
Expand Down
13 changes: 0 additions & 13 deletions src/test/ui/macros/macro-path-prelude-fail-3.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,3 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[derive(inline)] //~ ERROR cannot find derive macro `inline` in this scope
struct S;

fn main() {
inline!(); //~ ERROR cannot find macro `inline!` in this scope
}
10 changes: 2 additions & 8 deletions src/test/ui/macros/macro-path-prelude-fail-3.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
error: cannot find derive macro `inline` in this scope
--> $DIR/macro-path-prelude-fail-3.rs:11:10
|
LL | #[derive(inline)] //~ ERROR cannot find derive macro `inline` in this scope
| ^^^^^^

error: cannot find macro `inline!` in this scope
--> $DIR/macro-path-prelude-fail-3.rs:15:5
--> $DIR/macro-path-prelude-fail-3.rs:2:5
|
LL | inline!(); //~ ERROR cannot find macro `inline!` in this scope
| ^^^^^^ help: you could try the macro: `line`

error: aborting due to 2 previous errors
error: aborting due to previous error

4 changes: 4 additions & 0 deletions src/test/ui/macros/macro-path-prelude-fail-4.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#[derive(inline)] //~ ERROR expected a macro, found built-in attribute
struct S;

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/macros/macro-path-prelude-fail-4.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: expected a macro, found built-in attribute
--> $DIR/macro-path-prelude-fail-4.rs:1:10
|
LL | #[derive(inline)] //~ ERROR expected a macro, found built-in attribute
| ^^^^^^

error: aborting due to previous error

0 comments on commit f789b6b

Please sign in to comment.