Auto merge of #34239 - jseyfried:fix_macro_use_scope_regression, r=nrc

bors · web-flow · commit 7aba683c76f3 · 2016-06-16T06:37:18.000-07:00
Revert a change in the scope of macros imported from crates to fix a regression Fixes #34212. The regression was caused by #34032, which changed the scope of macros imported from extern crates to match the scope of macros imported from modules. r? @nrc
diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
@@ -928,6 +928,6 @@ impl SyntaxEnv {
     pub fn is_crate_root(&mut self) -> bool {
         // The first frame is pushed in `SyntaxEnv::new()` and the second frame is
         // pushed when folding the crate root pseudo-module (c.f. noop_fold_crate).
-        self.chain.len() == 2
+        self.chain.len() <= 2
     }
 }
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
@@ -41,6 +41,7 @@ trait MacroGenerable: Sized {
 
     // Fold this node or list of nodes using the given folder.
     fn fold_with<F: Folder>(self, folder: &mut F) -> Self;
+    fn visit_with<'v, V: Visitor<'v>>(&'v self, visitor: &mut V);
 
     // Return a placeholder expansion to allow compilation to continue after an erroring expansion.
     fn dummy(span: Span) -> Self;
@@ -50,7 +51,9 @@ trait MacroGenerable: Sized {
 }
 
 macro_rules! impl_macro_generable {
-    ($($ty:ty: $kind_name:expr, .$make:ident, $(.$fold:ident)* $(lift .$fold_elt:ident)*,
+    ($($ty:ty: $kind_name:expr, .$make:ident,
+               $(.$fold:ident)*  $(lift .$fold_elt:ident)*,
+               $(.$visit:ident)* $(lift .$visit_elt:ident)*,
                |$span:ident| $dummy:expr;)*) => { $(
         impl MacroGenerable for $ty {
             fn kind_name() -> &'static str { $kind_name }
@@ -59,21 +62,27 @@ macro_rules! impl_macro_generable {
                 $( folder.$fold(self) )*
                 $( self.into_iter().flat_map(|item| folder. $fold_elt (item)).collect() )*
             }
+            fn visit_with<'v, V: Visitor<'v>>(&'v self, visitor: &mut V) {
+                $( visitor.$visit(self) )*
+                $( for item in self.as_slice() { visitor. $visit_elt (item) } )*
+            }
             fn dummy($span: Span) -> Self { $dummy }
         }
     )* }
 }
 
 impl_macro_generable! {
-    P<ast::Expr>: "expression", .make_expr, .fold_expr, |span| DummyResult::raw_expr(span);
-    P<ast::Pat>:  "pattern",    .make_pat,  .fold_pat,  |span| P(DummyResult::raw_pat(span));
-    P<ast::Ty>:   "type",       .make_ty,   .fold_ty,   |span| DummyResult::raw_ty(span);
-    SmallVector<ast::ImplItem>:
-        "impl item", .make_impl_items, lift .fold_impl_item, |_span| SmallVector::zero();
-    SmallVector<P<ast::Item>>:
-        "item",      .make_items,      lift .fold_item,      |_span| SmallVector::zero();
+    P<ast::Pat>: "pattern", .make_pat, .fold_pat, .visit_pat, |span| P(DummyResult::raw_pat(span));
+    P<ast::Ty>:  "type",    .make_ty,  .fold_ty,  .visit_ty,  |span| DummyResult::raw_ty(span);
+    P<ast::Expr>:
+        "expression", .make_expr, .fold_expr, .visit_expr, |span| DummyResult::raw_expr(span);
     SmallVector<ast::Stmt>:
-        "statement", .make_stmts,      lift .fold_stmt,      |_span| SmallVector::zero();
+        "statement",  .make_stmts, lift .fold_stmt, lift .visit_stmt, |_span| SmallVector::zero();
+    SmallVector<P<ast::Item>>:
+        "item",       .make_items, lift .fold_item, lift .visit_item, |_span| SmallVector::zero();
+    SmallVector<ast::ImplItem>:
+        "impl item",  .make_impl_items, lift .fold_impl_item, lift .visit_impl_item,
+        |_span| SmallVector::zero();
 }
 
 impl MacroGenerable for Option<P<ast::Expr>> {
@@ -85,6 +94,9 @@ impl MacroGenerable for Option<P<ast::Expr>> {
     fn fold_with<F: Folder>(self, folder: &mut F) -> Self {
         self.and_then(|expr| folder.fold_opt_expr(expr))
     }
+    fn visit_with<'v, V: Visitor<'v>>(&'v self, visitor: &mut V) {
+        self.as_ref().map(|expr| visitor.visit_expr(expr));
+    }
 }
 
 pub fn expand_expr(expr: ast::Expr, fld: &mut MacroExpander) -> P<ast::Expr> {
@@ -320,6 +332,7 @@ fn expand_mac_invoc<T>(mac: ast::Mac, ident: Option<Ident>, attrs: Vec<ast::Attr
 
     let marked = expanded.fold_with(&mut Marker { mark: mark, expn_id: Some(fld.cx.backtrace()) });
     let configured = marked.fold_with(&mut fld.strip_unconfigured());
+    fld.load_macros(&configured);
     let fully_expanded = configured.fold_with(fld);
     fld.cx.bt_pop();
     fully_expanded
@@ -742,15 +755,6 @@ fn expand_annotatable(a: Annotatable,
                 }
                 result
             },
-            ast::ItemKind::ExternCrate(_) => {
-                // We need to error on `#[macro_use] extern crate` when it isn't at the
-                // crate root, because `$crate` won't work properly.
-                let allows_macros = fld.cx.syntax_env.is_crate_root();
-                for def in fld.cx.loader.load_crate(&it, allows_macros) {
-                    fld.cx.insert_macro(def);
-                }
-                SmallVector::one(it)
-            },
             _ => noop_fold_item(it, fld),
         }.into_iter().map(|i| Annotatable::Item(i)).collect(),
 
@@ -999,6 +1003,40 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                                &self.cx.parse_sess.span_diagnostic,
                                self.cx.feature_gated_cfgs)
     }
+
+    fn load_macros<T: MacroGenerable>(&mut self, node: &T) {
+        struct MacroLoadingVisitor<'a, 'b: 'a>{
+            cx: &'a mut ExtCtxt<'b>,
+            at_crate_root: bool,
+        }
+
+        impl<'a, 'b, 'v> Visitor<'v> for MacroLoadingVisitor<'a, 'b> {
+            fn visit_mac(&mut self, _: &'v ast::Mac) {}
+            fn visit_item(&mut self, item: &'v ast::Item) {
+                if let ast::ItemKind::ExternCrate(..) = item.node {
+                    // We need to error on `#[macro_use] extern crate` when it isn't at the
+                    // crate root, because `$crate` won't work properly.
+                    for def in self.cx.loader.load_crate(item, self.at_crate_root) {
+                        self.cx.insert_macro(def);
+                    }
+                } else {
+                    let at_crate_root = ::std::mem::replace(&mut self.at_crate_root, false);
+                    visit::walk_item(self, item);
+                    self.at_crate_root = at_crate_root;
+                }
+            }
+            fn visit_block(&mut self, block: &'v ast::Block) {
+                let at_crate_root = ::std::mem::replace(&mut self.at_crate_root, false);
+                visit::walk_block(self, block);
+                self.at_crate_root = at_crate_root;
+            }
+        }
+
+        node.visit_with(&mut MacroLoadingVisitor {
+            at_crate_root: self.cx.syntax_env.is_crate_root(),
+            cx: self.cx,
+        });
+    }
 }
 
 impl<'a, 'b> Folder for MacroExpander<'a, 'b> {
@@ -1142,7 +1180,7 @@ impl<'feat> ExpansionConfig<'feat> {
 
 pub fn expand_crate(mut cx: ExtCtxt,
                     user_exts: Vec<NamedSyntaxExtension>,
-                    c: Crate) -> (Crate, HashSet<Name>) {
+                    mut c: Crate) -> (Crate, HashSet<Name>) {
     if std_inject::no_core(&c) {
         cx.crate_root = None;
     } else if std_inject::no_std(&c) {
@@ -1157,6 +1195,10 @@ pub fn expand_crate(mut cx: ExtCtxt,
             expander.cx.syntax_env.insert(name, extension);
         }
 
+        let items = SmallVector::many(c.module.items);
+        expander.load_macros(&items);
+        c.module.items = items.into();
+
         let err_count = cx.parse_sess.span_diagnostic.err_count();
         let mut ret = expander.fold_crate(c);
         ret.exported_macros = expander.cx.exported_macros.clone();
diff --git a/src/libsyntax/util/small_vector.rs b/src/libsyntax/util/small_vector.rs
@@ -29,6 +29,16 @@ enum SmallVectorRepr<T> {
     Many(Vec<T>),
 }
 
+impl<T> Into<Vec<T>> for SmallVector<T> {
+    fn into(self) -> Vec<T> {
+        match self.repr {
+            Zero => Vec::new(),
+            One(t) => vec![t],
+            Many(vec) => vec,
+        }
+    }
+}
+
 impl<T> FromIterator<T> for SmallVector<T> {
     fn from_iter<I: IntoIterator<Item=T>>(iter: I) -> SmallVector<T> {
         let mut v = SmallVector::zero();
diff --git a/src/test/compile-fail/macro-use-scope.rs b/src/test/compile-fail/macro-use-scope.rs
@@ -8,12 +8,25 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![feature(rustc_private)]
-macro_rules! m {
-    () => { #[macro_use] extern crate syntax; }
+// aux-build:two_macros.rs
+
+#![feature(rustc_attrs)]
+#![allow(unused)]
+
+fn f() {
+    let _ = macro_one!();
 }
+#[macro_use(macro_one)] // Check that this macro is usable in the above function
+extern crate two_macros;
+
+macro_rules! m { () => {
+    fn g() {
+        macro_two!();
+    }
+    #[macro_use(macro_two)] // Check that this macro is usable in the above function
+    extern crate two_macros as _two_macros;
+} }
 m!();
 
-fn main() {
-    help!(); //~ ERROR unexpected end of macro invocation
-}
+#[rustc_error]
+fn main() {} //~ ERROR compilation successful

Original file line number	Diff line number	Diff line change
`@@ -928,6 +928,6 @@ impl SyntaxEnv {`
`928`	`928`	`pub fn is_crate_root(&mut self) -> bool {`
`929`	`929`	// The first frame is pushed in `SyntaxEnv::new()` and the second frame is
`930`	`930`	`// pushed when folding the crate root pseudo-module (c.f. noop_fold_crate).`
`931`		`- self.chain.len() == 2`
	`931`	`+ self.chain.len() <= 2`
`932`	`932`	`}`
`933`	`933`	`}`