From 62c7d78a9a39688e6445aefbd4fe1d051b7a9886 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 5 Sep 2018 22:43:11 +0300 Subject: [PATCH] resolve: Remove `unshadowable_attrs` --- src/librustc_resolve/lib.rs | 2 - src/librustc_resolve/macros.rs | 51 +++++++++---------- src/libsyntax/ext/base.rs | 2 - src/libsyntax_ext/lib.rs | 14 +---- src/test/ui/issues/issue-11692-2.rs | 3 +- src/test/ui/issues/issue-11692-2.stderr | 4 +- .../test-shadowing/test-cant-be-shadowed.rs | 5 ++ 7 files changed, 34 insertions(+), 47 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 98fbcd1fb183b..66f80af363227 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1449,7 +1449,6 @@ pub struct Resolver<'a, 'b: 'a> { macro_names: FxHashSet, builtin_macros: FxHashMap>, macro_use_prelude: FxHashMap>, - unshadowable_attrs: FxHashMap>, pub all_macros: FxHashMap, macro_map: FxHashMap>, macro_defs: FxHashMap, @@ -1767,7 +1766,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { macro_names: FxHashSet(), builtin_macros: FxHashMap(), macro_use_prelude: FxHashMap(), - unshadowable_attrs: FxHashMap(), all_macros: FxHashMap(), macro_map: FxHashMap(), invocations, diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index b4c772cb8c63f..fe0cb523a1580 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -220,23 +220,6 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> { } } - fn add_unshadowable_attr(&mut self, ident: ast::Ident, ext: Lrc) { - let def_id = DefId { - krate: BUILTIN_MACROS_CRATE, - index: DefIndex::from_array_index(self.macro_map.len(), - DefIndexAddressSpace::Low), - }; - let kind = ext.kind(); - self.macro_map.insert(def_id, ext); - let binding = self.arenas.alloc_name_binding(NameBinding { - kind: NameBindingKind::Def(Def::Macro(def_id, kind), false), - span: DUMMY_SP, - vis: ty::Visibility::Invisible, - expansion: Mark::root(), - }); - self.unshadowable_attrs.insert(ident.name, binding); - } - fn resolve_imports(&mut self) { ImportResolver { resolver: self }.resolve_imports() } @@ -493,14 +476,8 @@ impl<'a, 'cl> Resolver<'a, 'cl> { return def; } - if kind == MacroKind::Attr { - if let Some(ext) = self.unshadowable_attrs.get(&path[0].name) { - return Ok(ext.def()); - } - } - let legacy_resolution = self.resolve_legacy_scope( - path[0], invoc_id, invocation.parent_legacy_scope.get(), false + path[0], invoc_id, invocation.parent_legacy_scope.get(), false, kind == MacroKind::Attr ); let result = if let Some(legacy_binding) = legacy_resolution { Ok(legacy_binding.def()) @@ -643,7 +620,19 @@ impl<'a, 'cl> Resolver<'a, 'cl> { } WhereToResolve::MacroUsePrelude => { match self.macro_use_prelude.get(&ident.name).cloned() { - Some(binding) => Ok((binding, FromPrelude(true))), + Some(binding) => { + let mut result = Ok((binding, FromPrelude(true))); + // FIXME: Keep some built-in macros working even if they are + // shadowed by non-attribute macros imported with `macro_use`. + // We need to come up with some more principled approach instead. + if is_attr && (ident.name == "test" || ident.name == "bench") { + if let Def::Macro(_, MacroKind::Bang) = + binding.def_ignoring_ambiguity() { + result = Err(Determinacy::Determined); + } + } + result + } None => Err(Determinacy::Determined), } } @@ -811,8 +800,16 @@ impl<'a, 'cl> Resolver<'a, 'cl> { ident: Ident, invoc_id: Mark, invoc_parent_legacy_scope: LegacyScope<'a>, - record_used: bool) + record_used: bool, + is_attr: bool) -> Option<&'a NameBinding<'a>> { + if is_attr && (ident.name == "test" || ident.name == "bench") { + // FIXME: Keep some built-in macros working even if they are + // shadowed by user-defined `macro_rules`. + // We need to come up with some more principled approach instead. + return None; + } + let ident = ident.modern(); // This is *the* result, resolution from the scope closest to the resolved identifier. @@ -898,7 +895,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { let span = ident.span; let invocation = self.invocations[&invoc_id]; let legacy_resolution = self.resolve_legacy_scope( - ident, invoc_id, invocation.parent_legacy_scope.get(), true + ident, invoc_id, invocation.parent_legacy_scope.get(), true, kind == MacroKind::Attr ); let resolution = self.resolve_lexical_macro_path_segment( ident, MacroNS, invoc_id, true, true, kind == MacroKind::Attr, span diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 0e059bc4a6ce5..1ea710097661a 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -721,7 +721,6 @@ pub trait Resolver { fn visit_ast_fragment_with_placeholders(&mut self, mark: Mark, fragment: &AstFragment, derives: &[Mark]); fn add_builtin(&mut self, ident: ast::Ident, ext: Lrc); - fn add_unshadowable_attr(&mut self, ident: ast::Ident, ext: Lrc); fn resolve_imports(&mut self); // Resolves attribute and derive legacy macros from `#![plugin(..)]`. @@ -761,7 +760,6 @@ impl Resolver for DummyResolver { fn visit_ast_fragment_with_placeholders(&mut self, _invoc: Mark, _fragment: &AstFragment, _derives: &[Mark]) {} fn add_builtin(&mut self, _ident: ast::Ident, _ext: Lrc) {} - fn add_unshadowable_attr(&mut self, _ident: ast::Ident, _ext: Lrc) {} fn resolve_imports(&mut self) {} fn find_legacy_attr_invoc(&mut self, _attrs: &mut Vec, _allow_derive: bool) diff --git a/src/libsyntax_ext/lib.rs b/src/libsyntax_ext/lib.rs index e16f3b1ccb3f3..88af4a73a1515 100644 --- a/src/libsyntax_ext/lib.rs +++ b/src/libsyntax_ext/lib.rs @@ -72,18 +72,6 @@ pub fn register_builtins(resolver: &mut dyn syntax::ext::base::Resolver, enable_quotes: bool) { deriving::register_builtin_derives(resolver); - { - let mut register_unshadowable = |name, ext| { - resolver.add_unshadowable_attr(ast::Ident::with_empty_ctxt(name), Lrc::new(ext)); - }; - - register_unshadowable(Symbol::intern("test"), - MultiModifier(Box::new(test::expand_test))); - - register_unshadowable(Symbol::intern("bench"), - MultiModifier(Box::new(test::expand_bench))); - } - let mut register = |name, ext| { resolver.add_builtin(ast::Ident::with_empty_ctxt(name), Lrc::new(ext)); }; @@ -147,6 +135,8 @@ pub fn register_builtins(resolver: &mut dyn syntax::ext::base::Resolver, } register(Symbol::intern("test_case"), MultiModifier(Box::new(test_case::expand))); + register(Symbol::intern("test"), MultiModifier(Box::new(test::expand_test))); + register(Symbol::intern("bench"), MultiModifier(Box::new(test::expand_bench))); // format_args uses `unstable` things internally. register(Symbol::intern("format_args"), diff --git a/src/test/ui/issues/issue-11692-2.rs b/src/test/ui/issues/issue-11692-2.rs index 424cbd981c87d..2e94a27838e43 100644 --- a/src/test/ui/issues/issue-11692-2.rs +++ b/src/test/ui/issues/issue-11692-2.rs @@ -9,6 +9,5 @@ // except according to those terms. fn main() { - concat!(test!()); - //~^ error: cannot find macro `test!` in this scope + concat!(test!()); //~ ERROR `test` can only be used in attributes } diff --git a/src/test/ui/issues/issue-11692-2.stderr b/src/test/ui/issues/issue-11692-2.stderr index 51d6041e92220..186c59a61493d 100644 --- a/src/test/ui/issues/issue-11692-2.stderr +++ b/src/test/ui/issues/issue-11692-2.stderr @@ -1,7 +1,7 @@ -error: cannot find macro `test!` in this scope +error: `test` can only be used in attributes --> $DIR/issue-11692-2.rs:12:13 | -LL | concat!(test!()); +LL | concat!(test!()); //~ ERROR `test` can only be used in attributes | ^^^^ error: aborting due to previous error diff --git a/src/test/ui/test-shadowing/test-cant-be-shadowed.rs b/src/test/ui/test-shadowing/test-cant-be-shadowed.rs index 4b1a437818f87..4e24b17bdd587 100644 --- a/src/test/ui/test-shadowing/test-cant-be-shadowed.rs +++ b/src/test/ui/test-shadowing/test-cant-be-shadowed.rs @@ -16,3 +16,8 @@ #[test] fn foo(){} + +macro_rules! test { () => () } + +#[test] +fn bar() {}