Delay assignment of node ids until after expansion. Ensures that each AST node

has a unique id. Fixes numerous bugs in macro expansion and deriving. Add two
representative tests.

Fixes rust-lang#7971
Fixes rust-lang#6304
Fixes rust-lang#8367
Fixes rust-lang#8754
Fixes rust-lang#8852
Fixes rust-lang#2543
Fixes rust-lang#7654

Author: nikomatsakis

src/librustc/driver/driver.rs

@@ -190,6 +190,9 @@ pub fn phase_2_configure_and_expand(sess: Session,
     crate = time(time_passes, ~"std injection", ||
                  front::std_inject::maybe_inject_libstd_ref(sess, crate));
 
+    crate = time(time_passes, ~"assigning node ids", ||
+                 front::assign_node_ids::assign_node_ids(sess, crate));
+
     return crate;
 }
 
@@ -207,6 +210,7 @@ pub fn phase_3_run_analysis_passes(sess: Session,
                                    crate: @ast::Crate) -> CrateAnalysis {
 
     let time_passes = sess.time_passes();
+
     let ast_map = time(time_passes, ~"ast indexing", ||
                        syntax::ast_map::map_crate(sess.diagnostic(), crate));
 
@@ -812,6 +816,7 @@ pub fn build_session_(sopts: @session::options,
         building_library: @mut false,
         working_dir: os::getcwd(),
         lints: @mut HashMap::new(),
+        node_id: @mut 1
     }
 }
 

src/librustc/driver/session.rs

@@ -28,6 +28,7 @@ use syntax::abi;
 use syntax::parse::token;
 use syntax;
 
+use std::int;
 use std::hashmap::HashMap;
 
 #[deriving(Eq)]
@@ -216,57 +217,58 @@ pub struct Session_ {
     building_library: @mut bool,
     working_dir: Path,
     lints: @mut HashMap<ast::NodeId, ~[(lint::lint, codemap::Span, ~str)]>,
+    node_id: @mut uint,
 }
 
 pub type Session = @Session_;
 
 impl Session_ {
-    pub fn span_fatal(@self, sp: Span, msg: &str) -> ! {
+    pub fn span_fatal(&self, sp: Span, msg: &str) -> ! {
         self.span_diagnostic.span_fatal(sp, msg)
     }
-    pub fn fatal(@self, msg: &str) -> ! {
+    pub fn fatal(&self, msg: &str) -> ! {
         self.span_diagnostic.handler().fatal(msg)
     }
-    pub fn span_err(@self, sp: Span, msg: &str) {
+    pub fn span_err(&self, sp: Span, msg: &str) {
         self.span_diagnostic.span_err(sp, msg)
     }
-    pub fn err(@self, msg: &str) {
+    pub fn err(&self, msg: &str) {
         self.span_diagnostic.handler().err(msg)
     }
-    pub fn err_count(@self) -> uint {
+    pub fn err_count(&self) -> uint {
         self.span_diagnostic.handler().err_count()
     }
-    pub fn has_errors(@self) -> bool {
+    pub fn has_errors(&self) -> bool {
         self.span_diagnostic.handler().has_errors()
     }
-    pub fn abort_if_errors(@self) {
+    pub fn abort_if_errors(&self) {
         self.span_diagnostic.handler().abort_if_errors()
     }
-    pub fn span_warn(@self, sp: Span, msg: &str) {
+    pub fn span_warn(&self, sp: Span, msg: &str) {
         self.span_diagnostic.span_warn(sp, msg)
     }
-    pub fn warn(@self, msg: &str) {
+    pub fn warn(&self, msg: &str) {
         self.span_diagnostic.handler().warn(msg)
     }
-    pub fn span_note(@self, sp: Span, msg: &str) {
+    pub fn span_note(&self, sp: Span, msg: &str) {
         self.span_diagnostic.span_note(sp, msg)
     }
-    pub fn note(@self, msg: &str) {
+    pub fn note(&self, msg: &str) {
         self.span_diagnostic.handler().note(msg)
     }
-    pub fn span_bug(@self, sp: Span, msg: &str) -> ! {
+    pub fn span_bug(&self, sp: Span, msg: &str) -> ! {
         self.span_diagnostic.span_bug(sp, msg)
     }
-    pub fn bug(@self, msg: &str) -> ! {
+    pub fn bug(&self, msg: &str) -> ! {
         self.span_diagnostic.handler().bug(msg)
     }
-    pub fn span_unimpl(@self, sp: Span, msg: &str) -> ! {
+    pub fn span_unimpl(&self, sp: Span, msg: &str) -> ! {
         self.span_diagnostic.span_unimpl(sp, msg)
     }
-    pub fn unimpl(@self, msg: &str) -> ! {
+    pub fn unimpl(&self, msg: &str) -> ! {
         self.span_diagnostic.handler().unimpl(msg)
     }
-    pub fn add_lint(@self,
+    pub fn add_lint(&self,
                     lint: lint::lint,
                     id: ast::NodeId,
                     sp: Span,
@@ -277,77 +279,85 @@ impl Session_ {
         }
         self.lints.insert(id, ~[(lint, sp, msg)]);
     }
-    pub fn next_node_id(@self) -> ast::NodeId {
-        return syntax::parse::next_node_id(self.parse_sess);
+    pub fn next_node_id(&self) -> ast::NodeId {
+        self.reserve_node_ids(1)
     }
-    pub fn diagnostic(@self) -> @mut diagnostic::span_handler {
+    pub fn reserve_node_ids(&self, count: uint) -> ast::NodeId {
+        let v = *self.node_id;
+        *self.node_id += count;
+        if v > (int::max_value as uint) {
+            self.bug("Input too large, ran out of node ids!");
+        }
+        v as int
+    }
+    pub fn diagnostic(&self) -> @mut diagnostic::span_handler {
         self.span_diagnostic
     }
-    pub fn debugging_opt(@self, opt: uint) -> bool {
+    pub fn debugging_opt(&self, opt: uint) -> bool {
         (self.opts.debugging_opts & opt) != 0u
     }
     // This exists to help with refactoring to eliminate impossible
     // cases later on
-    pub fn impossible_case(@self, sp: Span, msg: &str) -> ! {
+    pub fn impossible_case(&self, sp: Span, msg: &str) -> ! {
         self.span_bug(sp, fmt!("Impossible case reached: %s", msg));
     }
-    pub fn verbose(@self) -> bool { self.debugging_opt(verbose) }
-    pub fn time_passes(@self) -> bool { self.debugging_opt(time_passes) }
-    pub fn count_llvm_insns(@self) -> bool {
+    pub fn verbose(&self) -> bool { self.debugging_opt(verbose) }
+    pub fn time_passes(&self) -> bool { self.debugging_opt(time_passes) }
+    pub fn count_llvm_insns(&self) -> bool {
         self.debugging_opt(count_llvm_insns)
     }
-    pub fn count_type_sizes(@self) -> bool {
+    pub fn count_type_sizes(&self) -> bool {
         self.debugging_opt(count_type_sizes)
     }
-    pub fn time_llvm_passes(@self) -> bool {
+    pub fn time_llvm_passes(&self) -> bool {
         self.debugging_opt(time_llvm_passes)
     }
-    pub fn trans_stats(@self) -> bool { self.debugging_opt(trans_stats) }
-    pub fn meta_stats(@self) -> bool { self.debugging_opt(meta_stats) }
-    pub fn asm_comments(@self) -> bool { self.debugging_opt(asm_comments) }
-    pub fn no_verify(@self) -> bool { self.debugging_opt(no_verify) }
-    pub fn lint_llvm(@self) -> bool { self.debugging_opt(lint_llvm) }
-    pub fn trace(@self) -> bool { self.debugging_opt(trace) }
-    pub fn coherence(@self) -> bool { self.debugging_opt(coherence) }
-    pub fn borrowck_stats(@self) -> bool { self.debugging_opt(borrowck_stats) }
-    pub fn borrowck_note_pure(@self) -> bool {
+    pub fn trans_stats(&self) -> bool { self.debugging_opt(trans_stats) }
+    pub fn meta_stats(&self) -> bool { self.debugging_opt(meta_stats) }
+    pub fn asm_comments(&self) -> bool { self.debugging_opt(asm_comments) }
+    pub fn no_verify(&self) -> bool { self.debugging_opt(no_verify) }
+    pub fn lint_llvm(&self) -> bool { self.debugging_opt(lint_llvm) }
+    pub fn trace(&self) -> bool { self.debugging_opt(trace) }
+    pub fn coherence(&self) -> bool { self.debugging_opt(coherence) }
+    pub fn borrowck_stats(&self) -> bool { self.debugging_opt(borrowck_stats) }
+    pub fn borrowck_note_pure(&self) -> bool {
         self.debugging_opt(borrowck_note_pure)
     }
-    pub fn borrowck_note_loan(@self) -> bool {
+    pub fn borrowck_note_loan(&self) -> bool {
         self.debugging_opt(borrowck_note_loan)
     }
-    pub fn no_monomorphic_collapse(@self) -> bool {
+    pub fn no_monomorphic_collapse(&self) -> bool {
         self.debugging_opt(no_monomorphic_collapse)
     }
-    pub fn debug_borrows(@self) -> bool {
+    pub fn debug_borrows(&self) -> bool {
         self.opts.optimize == No && !self.debugging_opt(no_debug_borrows)
     }
-    pub fn once_fns(@self) -> bool { self.debugging_opt(once_fns) }
-    pub fn print_llvm_passes(@self) -> bool {
+    pub fn once_fns(&self) -> bool { self.debugging_opt(once_fns) }
+    pub fn print_llvm_passes(&self) -> bool {
         self.debugging_opt(print_llvm_passes)
     }
-    pub fn no_prepopulate_passes(@self) -> bool {
+    pub fn no_prepopulate_passes(&self) -> bool {
         self.debugging_opt(no_prepopulate_passes)
     }
-    pub fn no_vectorize_loops(@self) -> bool {
+    pub fn no_vectorize_loops(&self) -> bool {
         self.debugging_opt(no_vectorize_loops)
     }
-    pub fn no_vectorize_slp(@self) -> bool {
+    pub fn no_vectorize_slp(&self) -> bool {
         self.debugging_opt(no_vectorize_slp)
     }
 
     // pointless function, now...
-    pub fn str_of(@self, id: ast::Ident) -> @str {
+    pub fn str_of(&self, id: ast::Ident) -> @str {
         token::ident_to_str(&id)
     }
 
     // pointless function, now...
-    pub fn ident_of(@self, st: &str) -> ast::Ident {
+    pub fn ident_of(&self, st: &str) -> ast::Ident {
         token::str_to_ident(st)
     }
 
     // pointless function, now...
-    pub fn intr(@self) -> @syntax::parse::token::ident_interner {
+    pub fn intr(&self) -> @syntax::parse::token::ident_interner {
         token::get_ident_interner()
     }
 }

src/librustc/front/assign_node_ids.rs

@@ -0,0 +1,19 @@
+// Copyright 2012 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.
+
+use driver::session::Session;
+
+use syntax::ast;
+use syntax::ast_util;
+
+pub fn assign_node_ids(sess: Session, crate: @ast::Crate) -> @ast::Crate {
+    let fold = ast_util::node_id_assigner(|| sess.next_node_id());
+    @fold.fold_crate(crate)
+}

src/librustc/front/std_inject.rs

@@ -45,7 +45,7 @@ fn inject_libstd_ref(sess: Session, crate: &ast::Crate) -> @ast::Crate {
 
     let precursor = @fold::AstFoldFns {
         fold_crate: |crate, fld| {
-            let n1 = sess.next_node_id();
+            let n1 = ast::DUMMY_NODE_ID;
             let vi1 = ast::view_item {
                 node: ast::view_item_extern_mod(
                         sess.ident_of("std"), None, ~[], n1),
@@ -86,7 +86,7 @@ fn inject_libstd_ref(sess: Session, crate: &ast::Crate) -> @ast::Crate {
             }
         },
         fold_mod: |module, fld| {
-            let n2 = sess.next_node_id();
+            let n2 = ast::DUMMY_NODE_ID;
 
             let prelude_path = ast::Path {
                 span: dummy_sp(),

src/librustc/front/test.rs

@@ -280,10 +280,10 @@ fn mk_std(cx: &TestCtxt) -> ast::view_item {
         ast::view_item_use(
             ~[@nospan(ast::view_path_simple(id_extra,
                                             path_node(~[id_extra]),
-                                            cx.sess.next_node_id()))])
+                                            ast::DUMMY_NODE_ID))])
     } else {
         let mi = attr::mk_name_value_item_str(@"vers", @"0.8-pre");
-        ast::view_item_extern_mod(id_extra, None, ~[mi], cx.sess.next_node_id())
+        ast::view_item_extern_mod(id_extra, None, ~[mi], ast::DUMMY_NODE_ID)
     };
     ast::view_item {
         node: vi,
@@ -325,7 +325,7 @@ fn mk_test_module(cx: &TestCtxt) -> @ast::item {
     let item = ast::item {
         ident: cx.sess.ident_of("__test"),
         attrs: ~[resolve_unexported_attr],
-        id: cx.sess.next_node_id(),
+        id: ast::DUMMY_NODE_ID,
         node: item_,
         vis: ast::public,
         span: dummy_sp(),
@@ -367,7 +367,7 @@ fn mk_test_module(cx: &TestCtxt) -> @ast::item {
     let item = ast::item {
         ident: cx.sess.ident_of("__test"),
         attrs: ~[resolve_unexported_attr],
-        id: cx.sess.next_node_id(),
+        id: ast::DUMMY_NODE_ID,
         node: item_,
         vis: ast::public,
         span: dummy_sp(),
@@ -448,15 +448,14 @@ fn mk_test_descs(cx: &TestCtxt) -> @ast::Expr {
         descs.push(mk_test_desc_and_fn_rec(cx, test));
     }
 
-    let sess = cx.sess;
     let inner_expr = @ast::Expr {
-        id: sess.next_node_id(),
+        id: ast::DUMMY_NODE_ID,
         node: ast::ExprVec(descs, ast::MutImmutable),
         span: dummy_sp(),
     };
 
     @ast::Expr {
-        id: sess.next_node_id(),
+        id: ast::DUMMY_NODE_ID,
         node: ast::ExprVstore(inner_expr, ast::ExprVstoreSlice),
         span: dummy_sp(),
     }
@@ -475,15 +474,15 @@ fn mk_test_desc_and_fn_rec(cx: &TestCtxt, test: &Test) -> @ast::Expr {
         nospan(ast::lit_str(ast_util::path_name_i(path).to_managed()));
 
     let name_expr = @ast::Expr {
-          id: cx.sess.next_node_id(),
+          id: ast::DUMMY_NODE_ID,
           node: ast::ExprLit(@name_lit),
           span: span
     };
 
     let fn_path = path_node_global(path);
 
     let fn_expr = @ast::Expr {
-        id: cx.sess.next_node_id(),
+        id: ast::DUMMY_NODE_ID,
         node: ast::ExprPath(fn_path),
         span: span,
     };
@@ -529,15 +528,15 @@ fn mk_test_desc_and_fn_rec(cx: &TestCtxt, test: &Test) -> @ast::Expr {
         nospan(ast::lit_str(ast_util::path_name_i(path).to_managed()));
 
     let name_expr = @ast::Expr {
-          id: cx.sess.next_node_id(),
+          id: ast::DUMMY_NODE_ID,
           node: ast::ExprLit(@name_lit),
           span: span
     };
 
     let fn_path = path_node_global(path);
 
     let fn_expr = @ast::Expr {
-        id: cx.sess.next_node_id(),
+        id: ast::DUMMY_NODE_ID,
         node: ast::ExprPath(fn_path),
         span: span,
     };

src/librustc/middle/astencode.rs

@@ -157,10 +157,10 @@ fn reserve_id_range(sess: Session,
     // Handle the case of an empty range:
     if from_id_range.empty() { return from_id_range; }
     let cnt = from_id_range.max - from_id_range.min;
-    let to_id_min = sess.parse_sess.next_id;
-    let to_id_max = sess.parse_sess.next_id + cnt;
-    sess.parse_sess.next_id = to_id_max;
-    ast_util::id_range { min: to_id_min, max: to_id_min }
+    assert!(cnt >= 0);
+    let to_id_min = sess.reserve_node_ids(cnt as uint);
+    let to_id_max = to_id_min + cnt;
+    ast_util::id_range { min: to_id_min, max: to_id_max }
 }
 
 impl ExtendedDecodeContext {

src/librustc/middle/resolve.rs

@@ -5403,7 +5403,15 @@ impl Resolver {
 
     pub fn record_def(@mut self, node_id: NodeId, def: Def) {
         debug!("(recording def) recording %? for %?", def, node_id);
-        self.def_map.insert(node_id, def);
+        do self.def_map.insert_or_update_with(node_id, def) |_, old_value| {
+            // Resolve appears to "resolve" the same ID multiple
+            // times, so here is a sanity check it at least comes to
+            // the same conclusion! - nmatsakis
+            if def != *old_value {
+                self.session.bug(fmt!("node_id %? resolved first to %? \
+                                      and then %?", node_id, *old_value, def));
+            }
+        };
     }
 
     pub fn enforce_default_binding_mode(@mut self,

src/librustc/middle/typeck/coherence.rs

@@ -45,7 +45,6 @@ use syntax::ast_util::{def_id_of_def, local_def};
 use syntax::codemap::Span;
 use syntax::opt_vec;
 use syntax::visit;
-use syntax::parse;
 
 use std::hashmap::HashSet;
 use std::result::Ok;
@@ -334,7 +333,7 @@ impl CoherenceChecker {
         let provided = ty::provided_trait_methods(tcx, trait_ref.def_id);
         for trait_method in provided.iter() {
             // Synthesize an ID.
-            let new_id = parse::next_node_id(tcx.sess.parse_sess);
+            let new_id = tcx.sess.next_node_id();
             let new_did = local_def(new_id);
 
             debug!("new_did=%? trait_method=%s", new_did, trait_method.repr(tcx));