Warn when macro shadows previous definition

src/libcore/macros.rs

@@ -9,6 +9,7 @@
 // except according to those terms.
 
 #![macro_escape]
+#![allow(shadowing_macros)]
 
 // NOTE(stage0): Remove cfg after a snapshot
 #[cfg(not(stage0))]

src/librustc/lint/builtin.rs

@@ -1627,6 +1627,52 @@ impl LintPass for MissingCopyImplementations {
     }
 }
 
+declare_lint! {
+    pub SHADOWING_MACROS,
+    Warn,
+    "detects shadowing macro definitions"
+}
+
+#[deriving(Copy)]
+pub struct ShadowingMacrosPass;
+
+impl LintPass for ShadowingMacrosPass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(SHADOWING_MACROS)
+    }
+
+    fn check_crate(&mut self, cx: &Context, krate: &ast::Crate) {
+        // Will map name uints to macro ast::Items. The loop below will then
+        // make a lint warning if a macro use with the name already exists
+        // in the map, or add that macro to the map so that subsequent ones
+        // with the same name will warn.
+        let mut uses = FnvHashMap::new();
+
+        for it in krate.macros.iter() {
+            let name = it.ident.name;
+            match uses.get(&name.uint()) {
+                Some(_) => {
+                    let span = it.span;
+                    // FIXME: Include span-printing of the first use to
+                    //        help users.
+                    cx.span_lint(
+                        SHADOWING_MACROS,
+                        span,
+                        format!(
+                            "shadowing macro definition: {}",
+                            name.as_str()
+                        )[]
+                    );
+                    continue;
+                },
+                None => { }
+            }
+            // Have to put None-case here to dodge the borrow-checker.
+            uses.insert(name.uint(), it);
+        }
+    }
+}
+
 declare_lint! {
     DEPRECATED,
     Warn,
@@ -1903,7 +1949,8 @@ impl LintPass for HardwiredLints {
             UNKNOWN_FEATURES,
             UNKNOWN_CRATE_TYPES,
             VARIANT_SIZE_DIFFERENCES,
-            FAT_PTR_TRANSMUTES
+            FAT_PTR_TRANSMUTES,
+            SHADOWING_MACROS
         )
     }
 }

src/libsyntax/ast.rs

@@ -453,6 +453,9 @@ pub struct Crate {
     pub attrs: Vec<Attribute>,
     pub config: CrateConfig,
     pub span: Span,
+    /// Macros defined inside the crate
+    pub macros: Vec<P<Item>>,
+    /// Macros exported from the crate via macro_export attribute
     pub exported_macros: Vec<P<Item>>
 }
 

src/libsyntax/ast_map/mod.rs

@@ -888,6 +888,7 @@ pub fn map_crate<'ast, F: FoldOps>(forest: &'ast mut Forest, fold_ops: F) -> Map
         },
         attrs: vec![],
         config: vec![],
+        macros: vec![],
         exported_macros: vec![],
         span: DUMMY_SP
     });

src/libsyntax/ext/base.rs

@@ -471,9 +471,14 @@ pub struct ExtCtxt<'a> {
     pub backtrace: ExpnId,
     pub ecfg: expand::ExpansionConfig,
 
-    pub mod_path: Vec<ast::Ident> ,
+    pub mod_path: Vec<ast::Ident>,
     pub trace_mac: bool,
+    /// These will eventually be moved into the ast::Crate's .exported_macros
+    /// by libsyntax::fold::noop_fold_crate.
     pub exported_macros: Vec<P<ast::Item>>,
+    /// All macros found during crate expansion. Also moved into the
+    /// ast::Crate so a macro-shadowing LintPass can later check them.
+    pub macros: Vec<P<ast::Item>>,
 
     pub syntax_env: SyntaxEnv,
     pub recursion_count: uint,
@@ -491,6 +496,7 @@ impl<'a> ExtCtxt<'a> {
             ecfg: ecfg,
             trace_mac: false,
             exported_macros: Vec::new(),
+            macros: Vec::new(),
             syntax_env: env,
             recursion_count: 0,
         }

src/libsyntax/ext/expand.rs

@@ -640,6 +640,9 @@ pub fn expand_item_mac(it: P<ast::Item>, fld: &mut MacroExpander)
             // need to be marked. Not that it could be marked anyway.
             // create issue to recommend refactoring here?
             fld.cx.syntax_env.insert(intern(name[]), ext);
+            // Keep track of this macro definition both within the crate
+            // context and (if applicable) in its exports.
+            fld.cx.macros.push(it.clone());
             if attr::contains_name(it.attrs[], "macro_export") {
                 fld.cx.exported_macros.push(it);
             }
@@ -1209,7 +1212,10 @@ pub fn expand_crate(parse_sess: &parse::ParseSess,
     }
 
     let mut ret = expander.fold_crate(c);
+    // Copy the list of all macro expansions and exported macros into
+    // the crate.
     ret.exported_macros = expander.cx.exported_macros.clone();
+    ret.macros = expander.cx.macros.clone();
     parse_sess.span_diagnostic.handler().abort_if_errors();
     return ret;
 }

src/libsyntax/ext/tt/macro_rules.rs

@@ -221,8 +221,8 @@ pub fn add_new_extension<'cx>(cx: &'cx mut ExtCtxt,
                               arg: Vec<ast::TokenTree> )
                               -> Box<MacResult+'cx> {
 
-    let lhs_nm =  gensym_ident("lhs");
-    let rhs_nm =  gensym_ident("rhs");
+    let lhs_nm = gensym_ident("lhs");
+    let rhs_nm = gensym_ident("rhs");
 
     // The pattern that macro_rules matches.
     // The grammar for macro_rules! is:

src/libsyntax/fold.rs

@@ -1116,7 +1116,7 @@ pub fn noop_fold_mod<T: Folder>(Mod {inner, view_items, items}: Mod, folder: &mu
     }
 }
 
-pub fn noop_fold_crate<T: Folder>(Crate {module, attrs, config, exported_macros, span}: Crate,
+pub fn noop_fold_crate<T: Folder>(Crate {module, attrs, config, macros, exported_macros, span}: Crate,
                                   folder: &mut T) -> Crate {
     let config = folder.fold_meta_items(config);
 
@@ -1151,6 +1151,7 @@ pub fn noop_fold_crate<T: Folder>(Crate {module, attrs, config, exported_macros,
         module: module,
         attrs: attrs,
         config: config,
+        macros: macros,
         exported_macros: exported_macros,
         span: span,
     }

src/libsyntax/parse/parser.rs

@@ -6188,7 +6188,8 @@ impl<'a> Parser<'a> {
             attrs: inner,
             config: self.cfg.clone(),
             span: mk_sp(lo, self.span.lo),
-            exported_macros: Vec::new(),
+            macros: Vec::new(),
+            exported_macros: Vec::new()
         }
     }