auto merge of #20190 : cmr/rust/gate-macro-args, r=alexcrichton

Uses the same approach as #17286 (and
subsequent changes making it more correct), where the visitor will skip any
pieces of the AST that are from "foreign code", where the spans don't line up,
indicating that that piece of code is due to a macro expansion.

If this breaks your code, read the error message to determine which feature
gate you should add to your crate.

Closes #18102

[breaking-change]

Author: bors

src/librand/lib.rs

@@ -271,7 +271,8 @@ pub trait Rng {
     /// let choices = [1i, 2, 4, 8, 16, 32];
     /// let mut rng = thread_rng();
     /// println!("{}", rng.choose(&choices));
-    /// assert_eq!(rng.choose(choices[..0]), None);
+    /// # // replace with slicing syntax when it's stable!
+    /// assert_eq!(rng.choose(choices.slice_to(0)), None);
     /// ```
     fn choose<'a, T>(&mut self, values: &'a [T]) -> Option<&'a T> {
         if values.is_empty() {

src/librustc/lint/builtin.rs

@@ -1682,33 +1682,7 @@ impl Stability {
     }
 
     fn is_internal(&self, cx: &Context, span: Span) -> bool {
-        // first, check if the given expression was generated by a macro or not
-        // we need to go back the expn_info tree to check only the arguments
-        // of the initial macro call, not the nested ones.
-        let mut expnid = span.expn_id;
-        let mut is_internal = false;
-        while cx.tcx.sess.codemap().with_expn_info(expnid, |expninfo| {
-            match expninfo {
-                Some(ref info) => {
-                    // save the parent expn_id for next loop iteration
-                    expnid = info.call_site.expn_id;
-                    if info.callee.span.is_none() {
-                        // it's a compiler built-in, we *really* don't want to mess with it
-                        // so we skip it, unless it was called by a regular macro, in which case
-                        // we will handle the caller macro next turn
-                        is_internal = true;
-                        true // continue looping
-                    } else {
-                        // was this expression from the current macro arguments ?
-                        is_internal = !( span.lo > info.call_site.lo &&
-                                         span.hi < info.call_site.hi );
-                        true // continue looping
-                    }
-                },
-                _ => false // stop looping
-            }
-        }) { /* empty while loop body */ }
-        return is_internal;
+        cx.tcx.sess.codemap().span_is_internal(span)
     }
 }
 

src/librustc_driver/driver.rs

@@ -178,21 +178,6 @@ pub fn phase_2_configure_and_expand(sess: &Session,
     *sess.crate_metadata.borrow_mut() =
         collect_crate_metadata(sess, krate.attrs[]);
 
-    time(time_passes, "gated feature checking", (), |_| {
-        let (features, unknown_features) =
-            syntax::feature_gate::check_crate(&sess.parse_sess.span_diagnostic, &krate);
-
-        for uf in unknown_features.iter() {
-            sess.add_lint(lint::builtin::UNKNOWN_FEATURES,
-                          ast::CRATE_NODE_ID,
-                          *uf,
-                          "unknown feature".to_string());
-        }
-
-        sess.abort_if_errors();
-        *sess.features.borrow_mut() = features;
-    });
-
     time(time_passes, "recursion limit", (), |_| {
         middle::recursion_limit::update_recursion_limit(sess, &krate);
     });
@@ -205,6 +190,23 @@ pub fn phase_2_configure_and_expand(sess: &Session,
     //
     // baz! should not use this definition unless foo is enabled.
 
+    time(time_passes, "gated macro checking", (), |_| {
+        let (features, unknown_features) =
+            syntax::feature_gate::check_crate_macros(sess.codemap(),
+                                                     &sess.parse_sess.span_diagnostic,
+                                                     &krate);
+        for uf in unknown_features.iter() {
+            sess.add_lint(lint::builtin::UNKNOWN_FEATURES,
+                          ast::CRATE_NODE_ID,
+                          *uf,
+                          "unknown feature".to_string());
+        }
+
+        // these need to be set "early" so that expansion sees `quote` if enabled.
+        *sess.features.borrow_mut() = features;
+        sess.abort_if_errors();
+    });
+
     krate = time(time_passes, "configuration 1", krate, |krate|
                  syntax::config::strip_unconfigured_items(sess.diagnostic(), krate));
 
@@ -289,6 +291,14 @@ pub fn phase_2_configure_and_expand(sess: &Session,
         }
     );
 
+    // Needs to go *after* expansion to be able to check the results of macro expansion.
+    time(time_passes, "complete gated feature checking", (), |_| {
+        syntax::feature_gate::check_crate(sess.codemap(),
+                                          &sess.parse_sess.span_diagnostic,
+                                          &krate);
+        sess.abort_if_errors();
+    });
+
     // JBC: make CFG processing part of expansion to avoid this problem:
 
     // strip again, in case expansion added anything with a #[cfg].

src/libsyntax/codemap.rs

@@ -563,6 +563,38 @@ impl CodeMap {
             ExpnId(i) => f(Some(&(*self.expansions.borrow())[i as uint]))
         }
     }
+
+    /// Check if a span is "internal" to a macro. This means that it is entirely generated by a
+    /// macro expansion and contains no code that was passed in as an argument.
+    pub fn span_is_internal(&self, span: Span) -> bool {
+        // first, check if the given expression was generated by a macro or not
+        // we need to go back the expn_info tree to check only the arguments
+        // of the initial macro call, not the nested ones.
+        let mut is_internal = false;
+        let mut expnid = span.expn_id;
+        while self.with_expn_info(expnid, |expninfo| {
+            match expninfo {
+                Some(ref info) => {
+                    // save the parent expn_id for next loop iteration
+                    expnid = info.call_site.expn_id;
+                    if info.callee.span.is_none() {
+                        // it's a compiler built-in, we *really* don't want to mess with it
+                        // so we skip it, unless it was called by a regular macro, in which case
+                        // we will handle the caller macro next turn
+                        is_internal = true;
+                        true // continue looping
+                    } else {
+                        // was this expression from the current macro arguments ?
+                        is_internal = !( span.lo > info.call_site.lo &&
+                                         span.hi < info.call_site.hi );
+                        true // continue looping
+                    }
+                },
+                _ => false // stop looping
+            }
+        }) { /* empty while loop body */ }
+        return is_internal;
+    }
 }
 
 #[cfg(test)]

src/libsyntax/feature_gate.rs

@@ -24,7 +24,7 @@ use ast::NodeId;
 use ast;
 use attr;
 use attr::AttrMetaMethods;
-use codemap::Span;
+use codemap::{CodeMap, Span};
 use diagnostic::SpanHandler;
 use visit;
 use visit::Visitor;
@@ -127,6 +127,7 @@ impl Features {
 struct Context<'a> {
     features: Vec<&'static str>,
     span_handler: &'a SpanHandler,
+    cm: &'a CodeMap,
 }
 
 impl<'a> Context<'a> {
@@ -144,7 +145,71 @@ impl<'a> Context<'a> {
     }
 }
 
-impl<'a, 'v> Visitor<'v> for Context<'a> {
+struct MacroVisitor<'a> {
+    context: &'a Context<'a>
+}
+
+impl<'a, 'v> Visitor<'v> for MacroVisitor<'a> {
+    fn visit_view_item(&mut self, i: &ast::ViewItem) {
+        match i.node {
+            ast::ViewItemExternCrate(..) => {
+                for attr in i.attrs.iter() {
+                    if attr.name().get() == "phase"{
+                        self.context.gate_feature("phase", attr.span,
+                                          "compile time crate loading is \
+                                           experimental and possibly buggy");
+                    }
+                }
+            },
+            _ => { }
+        }
+        visit::walk_view_item(self, i)
+    }
+
+    fn visit_mac(&mut self, macro: &ast::Mac) {
+        let ast::MacInvocTT(ref path, _, _) = macro.node;
+        let id = path.segments.last().unwrap().identifier;
+
+        if id == token::str_to_ident("macro_rules") {
+            self.context.gate_feature("macro_rules", path.span, "macro definitions are \
+                not stable enough for use and are subject to change");
+        }
+
+        else if id == token::str_to_ident("asm") {
+            self.context.gate_feature("asm", path.span, "inline assembly is not \
+                stable enough for use and is subject to change");
+        }
+
+        else if id == token::str_to_ident("log_syntax") {
+            self.context.gate_feature("log_syntax", path.span, "`log_syntax!` is not \
+                stable enough for use and is subject to change");
+        }
+
+        else if id == token::str_to_ident("trace_macros") {
+            self.context.gate_feature("trace_macros", path.span, "`trace_macros` is not \
+                stable enough for use and is subject to change");
+        }
+
+        else if id == token::str_to_ident("concat_idents") {
+            self.context.gate_feature("concat_idents", path.span, "`concat_idents` is not \
+                stable enough for use and is subject to change");
+        }
+    }
+}
+
+struct PostExpansionVisitor<'a> {
+    context: &'a Context<'a>
+}
+
+impl<'a> PostExpansionVisitor<'a> {
+    fn gate_feature(&self, feature: &str, span: Span, explain: &str) {
+        if !self.context.cm.span_is_internal(span) {
+            self.context.gate_feature(feature, span, explain)
+        }
+    }
+}
+
+impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> {
     fn visit_name(&mut self, sp: Span, name: ast::Name) {
         if !token::get_name(name).get().is_ascii() {
             self.gate_feature("non_ascii_idents", sp,
@@ -217,7 +282,7 @@ impl<'a, 'v> Visitor<'v> for Context<'a> {
             }
 
             ast::ItemImpl(_, _, _, _, ref items) => {
-                if attr::contains_name(i.attrs.as_slice(),
+                if attr::contains_name(i.attrs[],
                                        "unsafe_destructor") {
                     self.gate_feature("unsafe_destructor",
                                       i.span,
@@ -256,36 +321,6 @@ impl<'a, 'v> Visitor<'v> for Context<'a> {
         }
     }
 
-    fn visit_mac(&mut self, macro: &ast::Mac) {
-        let ast::MacInvocTT(ref path, _, _) = macro.node;
-        let id = path.segments.last().unwrap().identifier;
-
-        if id == token::str_to_ident("macro_rules") {
-            self.gate_feature("macro_rules", path.span, "macro definitions are \
-                not stable enough for use and are subject to change");
-        }
-
-        else if id == token::str_to_ident("asm") {
-            self.gate_feature("asm", path.span, "inline assembly is not \
-                stable enough for use and is subject to change");
-        }
-
-        else if id == token::str_to_ident("log_syntax") {
-            self.gate_feature("log_syntax", path.span, "`log_syntax!` is not \
-                stable enough for use and is subject to change");
-        }
-
-        else if id == token::str_to_ident("trace_macros") {
-            self.gate_feature("trace_macros", path.span, "`trace_macros` is not \
-                stable enough for use and is subject to change");
-        }
-
-        else if id == token::str_to_ident("concat_idents") {
-            self.gate_feature("concat_idents", path.span, "`concat_idents` is not \
-                stable enough for use and is subject to change");
-        }
-    }
-
     fn visit_foreign_item(&mut self, i: &ast::ForeignItem) {
         if attr::contains_name(i.attrs[], "linkage") {
             self.gate_feature("linkage", i.span,
@@ -371,10 +406,15 @@ impl<'a, 'v> Visitor<'v> for Context<'a> {
     }
 }
 
-pub fn check_crate(span_handler: &SpanHandler, krate: &ast::Crate) -> (Features, Vec<Span>) {
+fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate,
+                        check: F)
+                       -> (Features, Vec<Span>)
+    where F: FnOnce(&mut Context, &ast::Crate)
+{
     let mut cx = Context {
         features: Vec::new(),
         span_handler: span_handler,
+        cm: cm,
     };
 
     let mut unknown_features = Vec::new();
@@ -419,7 +459,7 @@ pub fn check_crate(span_handler: &SpanHandler, krate: &ast::Crate) -> (Features,
         }
     }
 
-    visit::walk_crate(&mut cx, krate);
+    check(&mut cx, krate);
 
     (Features {
         default_type_params: cx.has_feature("default_type_params"),
@@ -432,3 +472,16 @@ pub fn check_crate(span_handler: &SpanHandler, krate: &ast::Crate) -> (Features,
     },
     unknown_features)
 }
+
+pub fn check_crate_macros(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate)
+-> (Features, Vec<Span>) {
+    check_crate_inner(cm, span_handler, krate,
+                      |ctx, krate| visit::walk_crate(&mut MacroVisitor { context: ctx }, krate))
+}
+
+pub fn check_crate(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate)
+-> (Features, Vec<Span>) {
+    check_crate_inner(cm, span_handler, krate,
+                      |ctx, krate| visit::walk_crate(&mut PostExpansionVisitor { context: ctx },
+                                                     krate))
+}

src/test/compile-fail/feature-gated-feature-in-macro-arg.rs

@@ -0,0 +1,24 @@
+// Copyright 2014 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.
+
+// tests that input to a macro is checked for use of gated features. If this
+// test succeeds due to the acceptance of a feature, pick a new feature to
+// test. Not ideal, but oh well :(
+
+fn main() {
+    let a = &[1i32, 2, 3];
+    println!("{}", {
+        extern "rust-intrinsic" { //~ ERROR intrinsics are subject to change
+            fn atomic_fence();
+        }
+        atomic_fence();
+        42
+    });
+}