From 78ec6fe30cf2b1e85db76107154d315fde6af8bd Mon Sep 17 00:00:00 2001
From: Tim Chevalier <chevalier@alum.wellesley.edu>
Date: Wed, 11 Jul 2012 14:31:35 -0700
Subject: [PATCH] Obliterate the callee_id hack

Exprs that could be applications of overloaded operators
(expr_unary, expr_binary, expr_index) relied on the previous node ID
being "reserved" to carry extra typechecking info. This was
incredibly error-prone. Fixed it; now all exprs have two node IDs
(which will be wasted in some cases; future work could make this
an option instead if the extra int field ends up being a performance
problem).

Closes #2804
---
 src/fuzzer/fuzzer.rs                       |  2 +-
 src/libsyntax/ast.rs                       |  3 +-
 src/libsyntax/ast_util.rs                  | 13 +---
 src/libsyntax/ext/auto_serialize.rs        |  3 +-
 src/libsyntax/ext/build.rs                 |  7 ++-
 src/libsyntax/ext/concat_idents.rs         |  1 +
 src/libsyntax/ext/log_syntax.rs            |  4 +-
 src/libsyntax/ext/simplext.rs              |  2 +-
 src/libsyntax/fold.rs                      |  1 +
 src/libsyntax/parse/parser.rs              | 10 +++-
 src/rustc/front/test.rs                    | 28 ++++++---
 src/rustc/middle/borrowck.rs               |  1 -
 src/rustc/middle/borrowck/check_loans.rs   |  4 +-
 src/rustc/middle/lint.rs                   |  1 +
 src/rustc/middle/trans/base.rs             | 24 ++++----
 src/rustc/middle/typeck/check.rs           | 15 +++--
 src/rustc/middle/typeck/check/method.rs    |  2 +-
 src/rustc/middle/typeck/check/vtable.rs    |  2 +-
 src/rustc/middle/typeck/check/writeback.rs |  3 +-
 src/test/run-pass/issue-2804-2.rs          | 10 ++++
 src/test/run-pass/issue-2804.rs            | 69 ++++++++++++++++++++++
 21 files changed, 148 insertions(+), 57 deletions(-)
 create mode 100644 src/test/run-pass/issue-2804-2.rs
 create mode 100644 src/test/run-pass/issue-2804.rs

diff --git a/src/fuzzer/fuzzer.rs b/src/fuzzer/fuzzer.rs
index a0621bf257f..00a9ceeabd7 100644
--- a/src/fuzzer/fuzzer.rs
+++ b/src/fuzzer/fuzzer.rs
@@ -34,7 +34,7 @@ fn find_rust_files(&files: ~[str], path: str) {
 
 fn common_exprs() -> ~[ast::expr] {
     fn dse(e: ast::expr_) -> ast::expr {
-        { id: 0, node: e, span: ast_util::dummy_sp() }
+        { id: 0, callee_id: -1, node: e, span: ast_util::dummy_sp() }
     }
 
     fn dsl(l: ast::lit_) -> ast::lit {
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index 10a29453c1e..e79e5e2ab77 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -288,7 +288,8 @@ enum blk_check_mode { default_blk, unchecked_blk, unsafe_blk, }
 enum expr_check_mode { claimed_expr, checked_expr, }
 
 #[auto_serialize]
-type expr = {id: node_id, node: expr_, span: span};
+type expr = {id: node_id, callee_id: node_id, node: expr_, span: span};
+// Extra node ID is only used for index, assign_op, unary, binary
 
 #[auto_serialize]
 enum alt_mode { alt_check, alt_exhaustive, }
diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs
index 806f6c35ed0..1e2de4f5bfa 100644
--- a/src/libsyntax/ast_util.rs
+++ b/src/libsyntax/ast_util.rs
@@ -272,11 +272,6 @@ fn ident_to_path(s: span, +i: ident) -> @path {
     if is_unguarded(a) { some(/* FIXME (#2543) */ copy a.pats) } else { none }
 }
 
-// Provides an extra node_id to hang callee information on, in case the
-// operator is deferred to a user-supplied method. The parser is responsible
-// for reserving this id.
-fn op_expr_callee_id(e: @expr) -> node_id { e.id - 1 }
-
 pure fn class_item_ident(ci: @class_member) -> ident {
     alt ci.node {
       instance_var(i,_,_,_,_) { /* FIXME (#2543) */ copy i }
@@ -455,14 +450,8 @@ fn id_visitor(vfn: fn@(node_id)) -> visit::vt<()> {
         },
 
         visit_expr: fn@(e: @expr) {
+            vfn(e.callee_id);
             vfn(e.id);
-            alt e.node {
-              expr_index(*) | expr_assign_op(*) |
-              expr_unary(*) | expr_binary(*) {
-                vfn(ast_util::op_expr_callee_id(e));
-              }
-              _ { /* fallthrough */ }
-            }
         },
 
         visit_ty: fn@(t: @ty) {
diff --git a/src/libsyntax/ext/auto_serialize.rs b/src/libsyntax/ext/auto_serialize.rs
index e32ebf4eae5..119dbb59109 100644
--- a/src/libsyntax/ext/auto_serialize.rs
+++ b/src/libsyntax/ext/auto_serialize.rs
@@ -172,7 +172,8 @@ fn ty_nil(span: span) -> @ast::ty {
     }
 
     fn expr(span: span, node: ast::expr_) -> @ast::expr {
-        @{id: self.next_id(), node: node, span: span}
+        @{id: self.next_id(), callee_id: self.next_id(),
+          node: node, span: span}
     }
 
     fn var_ref(span: span, name: ast::ident) -> @ast::expr {
diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs
index 872d1f5eff6..342ecac2262 100644
--- a/src/libsyntax/ext/build.rs
+++ b/src/libsyntax/ext/build.rs
@@ -3,12 +3,13 @@
 
 fn mk_expr(cx: ext_ctxt, sp: codemap::span, expr: ast::expr_) ->
     @ast::expr {
-    ret @{id: cx.next_id(), node: expr, span: sp};
+    ret @{id: cx.next_id(), callee_id: cx.next_id(),
+          node: expr, span: sp};
 }
 
 fn mk_lit(cx: ext_ctxt, sp: span, lit: ast::lit_) -> @ast::expr {
     let sp_lit = @{node: lit, span: sp};
-    ret @{id: cx.next_id(), node: ast::expr_lit(sp_lit), span: sp};
+    mk_expr(cx, sp, ast::expr_lit(sp_lit))
 }
 fn mk_str(cx: ext_ctxt, sp: span, s: str) -> @ast::expr {
     let lit = ast::lit_str(@s);
@@ -62,7 +63,7 @@ fn mk_call(cx: ext_ctxt, sp: span, fn_path: ~[ast::ident],
 fn mk_base_vec_e(cx: ext_ctxt, sp: span, exprs: ~[@ast::expr]) ->
    @ast::expr {
     let vecexpr = ast::expr_vec(exprs, ast::m_imm);
-    ret @{id: cx.next_id(), node: vecexpr, span: sp};
+    mk_expr(cx, sp, vecexpr)
 }
 fn mk_vstore_e(cx: ext_ctxt, sp: span, expr: @ast::expr, vst: ast::vstore) ->
    @ast::expr {
diff --git a/src/libsyntax/ext/concat_idents.rs b/src/libsyntax/ext/concat_idents.rs
index a678304725d..f5d13df75d6 100644
--- a/src/libsyntax/ext/concat_idents.rs
+++ b/src/libsyntax/ext/concat_idents.rs
@@ -9,6 +9,7 @@ fn expand_syntax_ext(cx: ext_ctxt, sp: codemap::span, arg: ast::mac_arg,
     }
 
     ret @{id: cx.next_id(),
+          callee_id: cx.next_id(),
           node: ast::expr_path(@{span: sp, global: false, idents: ~[@res],
                                  rp: none, types: ~[]}),
           span: sp};
diff --git a/src/libsyntax/ext/log_syntax.rs b/src/libsyntax/ext/log_syntax.rs
index d237cd33839..06941fc5d38 100644
--- a/src/libsyntax/ext/log_syntax.rs
+++ b/src/libsyntax/ext/log_syntax.rs
@@ -11,6 +11,6 @@ fn expand_syntax_ext(cx: ext_ctxt, sp: codemap::span, arg: ast::mac_arg,
     );
 
     //trivial expression
-    ret @{id: cx.next_id(), node: ast::expr_rec(~[], option::none),
-          span: sp};
+    ret @{id: cx.next_id(), callee_id: cx.next_id(),
+          node: ast::expr_rec(~[], option::none), span: sp};
 }
diff --git a/src/libsyntax/ext/simplext.rs b/src/libsyntax/ext/simplext.rs
index 0d415ccfc43..6ccbabd748e 100644
--- a/src/libsyntax/ext/simplext.rs
+++ b/src/libsyntax/ext/simplext.rs
@@ -7,7 +7,7 @@
 import fold::*;
 import ast_util::respan;
 import ast::{ident, path, ty, blk_, expr, expr_path,
-             expr_vec, expr_mac, mac_invoc, node_id};
+             expr_vec, expr_mac, mac_invoc, node_id, expr_index};
 
 export add_new_extension;
 
diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs
index 21ae0de60ec..0e16d4bdca8 100644
--- a/src/libsyntax/fold.rs
+++ b/src/libsyntax/fold.rs
@@ -688,6 +688,7 @@ fn fold_decl(&&x: @decl) -> @decl {
     fn fold_expr(&&x: @expr) -> @expr {
         let (n, s) = self.fold_expr(x.node, x.span, self as ast_fold);
         ret @{id: self.new_id(x.id),
+              callee_id: self.new_id(x.callee_id),
               node: n,
               span: self.new_span(s)};
     }
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 9d8458c9c60..4a013f20dfd 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -1,3 +1,5 @@
+import print::pprust::expr_to_str;
+
 import result::result;
 import either::{either, left, right};
 import std::map::{hashmap, str_hash};
@@ -758,11 +760,13 @@ fn parse_field(sep: token::token) -> field {
     }
 
     fn mk_expr(lo: uint, hi: uint, +node: expr_) -> @expr {
-        ret @{id: self.get_id(), node: node, span: mk_sp(lo, hi)};
+        ret @{id: self.get_id(), callee_id: self.get_id(),
+              node: node, span: mk_sp(lo, hi)};
     }
 
     fn mk_mac_expr(lo: uint, hi: uint, m: mac_) -> @expr {
         ret @{id: self.get_id(),
+              callee_id: self.get_id(),
               node: expr_mac({node: m, span: mk_sp(lo, hi)}),
               span: mk_sp(lo, hi)};
     }
@@ -772,7 +776,8 @@ fn mk_lit_u32(i: u32) -> @expr {
         let lv_lit = @{node: lit_uint(i as u64, ty_u32),
                        span: span};
 
-        ret @{id: self.get_id(), node: expr_lit(lv_lit), span: span};
+        ret @{id: self.get_id(), callee_id: self.get_id(),
+              node: expr_lit(lv_lit), span: span};
     }
 
     fn mk_pexpr(lo: uint, hi: uint, node: expr_) -> pexpr {
@@ -1112,7 +1117,6 @@ fn parse_dot_or_call_expr_with(e0: pexpr) -> pexpr {
                 let ix = self.parse_expr();
                 hi = ix.span.hi;
                 self.expect(token::RBRACKET);
-                self.get_id(); // see ast_util::op_expr_callee_id
                 e = self.mk_pexpr(lo, hi, expr_index(self.to_expr(e), ix));
               }
 
diff --git a/src/rustc/front/test.rs b/src/rustc/front/test.rs
index 5f7a3b32265..44d68a3fcd9 100644
--- a/src/rustc/front/test.rs
+++ b/src/rustc/front/test.rs
@@ -283,9 +283,11 @@ fn mk_test_desc_vec(cx: test_ctxt) -> @ast::expr {
     }
 
     let inner_expr = @{id: cx.sess.next_node_id(),
+                       callee_id: cx.sess.next_node_id(),
                        node: ast::expr_vec(descs, ast::m_imm),
                        span: dummy_sp()};
     ret @{id: cx.sess.next_node_id(),
+          callee_id: cx.sess.next_node_id(),
           node: ast::expr_vstore(inner_expr, ast::vstore_uniq),
           span: dummy_sp()};
 }
@@ -300,6 +302,7 @@ fn mk_test_desc_rec(cx: test_ctxt, test: test) -> @ast::expr {
         nospan(ast::lit_str(@ast_util::path_name_i(path)));
     let name_expr: ast::expr =
         {id: cx.sess.next_node_id(),
+         callee_id: cx.sess.next_node_id(),
          node: ast::expr_lit(@name_lit),
          span: span};
 
@@ -310,6 +313,7 @@ fn mk_test_desc_rec(cx: test_ctxt, test: test) -> @ast::expr {
 
     let fn_expr: ast::expr =
         {id: cx.sess.next_node_id(),
+         callee_id: cx.sess.next_node_id(),
          node: ast::expr_path(fn_path),
          span: span};
 
@@ -322,6 +326,7 @@ fn mk_test_desc_rec(cx: test_ctxt, test: test) -> @ast::expr {
 
     let ignore_expr: ast::expr =
         {id: cx.sess.next_node_id(),
+         callee_id: cx.sess.next_node_id(),
          node: ast::expr_lit(@ignore_lit),
          span: span};
 
@@ -332,6 +337,7 @@ fn mk_test_desc_rec(cx: test_ctxt, test: test) -> @ast::expr {
 
     let fail_expr: ast::expr =
         {id: cx.sess.next_node_id(),
+         callee_id: cx.sess.next_node_id(),
          node: ast::expr_lit(@fail_lit),
          span: span};
 
@@ -342,7 +348,8 @@ fn mk_test_desc_rec(cx: test_ctxt, test: test) -> @ast::expr {
         ast::expr_rec(~[name_field, fn_field, ignore_field, fail_field],
             option::none);
     let desc_rec: ast::expr =
-        {id: cx.sess.next_node_id(), node: desc_rec_, span: span};
+        {id: cx.sess.next_node_id(), callee_id: cx.sess.next_node_id(),
+         node: desc_rec_, span: span};
     ret @desc_rec;
 }
 
@@ -354,6 +361,7 @@ fn mk_test_wrapper(cx: test_ctxt,
                    span: span) -> @ast::expr {
     let call_expr: ast::expr = {
         id: cx.sess.next_node_id(),
+        callee_id: cx.sess.next_node_id(),
         node: ast::expr_call(@fn_path_expr, ~[], false),
         span: span
     };
@@ -379,6 +387,7 @@ fn mk_test_wrapper(cx: test_ctxt,
 
     let wrapper_expr: ast::expr = {
         id: cx.sess.next_node_id(),
+        callee_id: cx.sess.next_node_id(),
         node: ast::expr_fn(ast::proto_bare, wrapper_decl,
                            wrapper_body, @~[]),
         span: span
@@ -444,7 +453,8 @@ fn mk_test_main_call(cx: test_ctxt) -> @ast::expr {
     let args_path_expr_: ast::expr_ = ast::expr_path(args_path);
 
     let args_path_expr: ast::expr =
-        {id: cx.sess.next_node_id(), node: args_path_expr_, span: dummy_sp()};
+        {id: cx.sess.next_node_id(), callee_id: cx.sess.next_node_id(),
+         node: args_path_expr_, span: dummy_sp()};
 
     // Call __test::test to generate the vector of test_descs
     let test_path = path_node(~[@"tests"]);
@@ -452,12 +462,14 @@ fn mk_test_main_call(cx: test_ctxt) -> @ast::expr {
     let test_path_expr_: ast::expr_ = ast::expr_path(test_path);
 
     let test_path_expr: ast::expr =
-        {id: cx.sess.next_node_id(), node: test_path_expr_, span: dummy_sp()};
+        {id: cx.sess.next_node_id(), callee_id: cx.sess.next_node_id(),
+         node: test_path_expr_, span: dummy_sp()};
 
     let test_call_expr_ = ast::expr_call(@test_path_expr, ~[], false);
 
     let test_call_expr: ast::expr =
-        {id: cx.sess.next_node_id(), node: test_call_expr_, span: dummy_sp()};
+        {id: cx.sess.next_node_id(), callee_id: cx.sess.next_node_id(),
+         node: test_call_expr_, span: dummy_sp()};
 
     // Call std::test::test_main
     let test_main_path = path_node(mk_path(cx, ~[@"test", @"test_main"]));
@@ -465,16 +477,16 @@ fn mk_test_main_call(cx: test_ctxt) -> @ast::expr {
     let test_main_path_expr_: ast::expr_ = ast::expr_path(test_main_path);
 
     let test_main_path_expr: ast::expr =
-        {id: cx.sess.next_node_id(), node: test_main_path_expr_,
-         span: dummy_sp()};
+        {id: cx.sess.next_node_id(), callee_id: cx.sess.next_node_id(),
+         node: test_main_path_expr_, span: dummy_sp()};
 
     let test_main_call_expr_: ast::expr_ =
         ast::expr_call(@test_main_path_expr,
                        ~[@args_path_expr, @test_call_expr], false);
 
     let test_main_call_expr: ast::expr =
-        {id: cx.sess.next_node_id(), node: test_main_call_expr_,
-         span: dummy_sp()};
+        {id: cx.sess.next_node_id(), callee_id: cx.sess.next_node_id(),
+         node: test_main_call_expr_, span: dummy_sp()};
 
     ret @test_main_call_expr;
 }
diff --git a/src/rustc/middle/borrowck.rs b/src/rustc/middle/borrowck.rs
index f08c9e9d4c2..01d04fa167a 100644
--- a/src/rustc/middle/borrowck.rs
+++ b/src/rustc/middle/borrowck.rs
@@ -159,7 +159,6 @@
 import result::{result, ok, err, extensions};
 import syntax::print::pprust;
 import util::common::indenter;
-import ast_util::op_expr_callee_id;
 import ty::to_str;
 import driver::session::session;
 import dvec::{dvec, extensions};
diff --git a/src/rustc/middle/borrowck/check_loans.rs b/src/rustc/middle/borrowck/check_loans.rs
index 4b80e934416..4977af45b7e 100644
--- a/src/rustc/middle/borrowck/check_loans.rs
+++ b/src/rustc/middle/borrowck/check_loans.rs
@@ -616,7 +616,7 @@ fn check_loans_in_expr(expr: @ast::expr,
       if self.bccx.method_map.contains_key(expr.id) {
         self.check_call(expr,
                         none,
-                        ast_util::op_expr_callee_id(expr),
+                        expr.callee_id,
                         expr.span,
                         ~[rval]);
       }
@@ -624,7 +624,7 @@ fn check_loans_in_expr(expr: @ast::expr,
       if self.bccx.method_map.contains_key(expr.id) {
         self.check_call(expr,
                         none,
-                        ast_util::op_expr_callee_id(expr),
+                        expr.callee_id,
                         expr.span,
                         ~[]);
       }
diff --git a/src/rustc/middle/lint.rs b/src/rustc/middle/lint.rs
index 2e33fe8dd9a..60c75a06dbf 100644
--- a/src/rustc/middle/lint.rs
+++ b/src/rustc/middle/lint.rs
@@ -405,6 +405,7 @@ fn check_item_path_statement(cx: ty::ctxt, it: @ast::item) {
         visit_stmt: fn@(s: @ast::stmt) {
             alt s.node {
               ast::stmt_semi(@{id: id,
+                               callee_id: _,
                                node: ast::expr_path(@path),
                                span: _}, _) {
                 cx.sess.span_lint(
diff --git a/src/rustc/middle/trans/base.rs b/src/rustc/middle/trans/base.rs
index eba5d551556..2b4447d71ce 100644
--- a/src/rustc/middle/trans/base.rs
+++ b/src/rustc/middle/trans/base.rs
@@ -1475,12 +1475,12 @@ fn trans_unary(bcx: block, op: ast::unop, e: @ast::expr,
     // Check for user-defined method call
     alt bcx.ccx().maps.method_map.find(un_expr.id) {
       some(mentry) {
-        let callee_id = ast_util::op_expr_callee_id(un_expr);
-        let fty = node_id_type(bcx, callee_id);
+        let fty = node_id_type(bcx, un_expr.callee_id);
         ret trans_call_inner(
             bcx, un_expr.info(), fty,
             expr_ty(bcx, un_expr),
-            |bcx| impl::trans_method_callee(bcx, callee_id, e, mentry),
+            |bcx| impl::trans_method_callee(bcx, un_expr.callee_id, e,
+                                            mentry),
             arg_exprs(~[]), dest);
       }
       _ {}
@@ -1703,10 +1703,9 @@ fn trans_assign_op(bcx: block, ex: @ast::expr, op: ast::binop,
     alt bcx.ccx().maps.method_map.find(ex.id) {
       some(origin) {
         let bcx = lhs_res.bcx;
-        let callee_id = ast_util::op_expr_callee_id(ex);
         #debug["user-defined method callee_id: %s",
-               ast_map::node_id_to_str(bcx.tcx().items, callee_id)];
-        let fty = node_id_type(bcx, callee_id);
+               ast_map::node_id_to_str(bcx.tcx().items, ex.callee_id)];
+        let fty = node_id_type(bcx, ex.callee_id);
 
         let dty = expr_ty(bcx, dst);
         let target = alloc_ty(bcx, dty);
@@ -1717,7 +1716,7 @@ fn trans_assign_op(bcx: block, ex: @ast::expr, op: ast::binop,
             |bcx| {
                 // FIXME (#2528): provide the already-computed address, not
                 // the expr.
-                impl::trans_method_callee(bcx, callee_id, dst, origin)
+                impl::trans_method_callee(bcx, ex.callee_id, dst, origin)
             },
             arg_exprs(~[src]), save_in(target));
 
@@ -1851,13 +1850,12 @@ fn trans_binary(bcx: block, op: ast::binop, lhs: @ast::expr,
     // User-defined operators
     alt bcx.ccx().maps.method_map.find(ex.id) {
       some(origin) {
-        let callee_id = ast_util::op_expr_callee_id(ex);
-        let fty = node_id_type(bcx, callee_id);
+        let fty = node_id_type(bcx, ex.callee_id);
         ret trans_call_inner(
             bcx, ex.info(), fty,
             expr_ty(bcx, ex),
             |bcx| {
-                impl::trans_method_callee(bcx, callee_id, lhs, origin)
+                impl::trans_method_callee(bcx, ex.callee_id, lhs, origin)
             },
             arg_exprs(~[rhs]), dest);
       }
@@ -3597,12 +3595,12 @@ fn unrooted(bcx: block, e: @ast::expr, dest: dest) -> block {
             // If it is here, it's not an lval, so this is a user-defined
             // index op
             let origin = bcx.ccx().maps.method_map.get(e.id);
-            let callee_id = ast_util::op_expr_callee_id(e);
-            let fty = node_id_type(bcx, callee_id);
+            let fty = node_id_type(bcx, e.callee_id);
             ret trans_call_inner(
                 bcx, e.info(), fty,
                 expr_ty(bcx, e),
-                |bcx| impl::trans_method_callee(bcx, callee_id, base, origin),
+                |bcx| impl::trans_method_callee(bcx, e.callee_id, base,
+                                                origin),
                 arg_exprs(~[idx]), dest);
           }
 
diff --git a/src/rustc/middle/typeck/check.rs b/src/rustc/middle/typeck/check.rs
index 063c1714192..43dc46cd2b6 100644
--- a/src/rustc/middle/typeck/check.rs
+++ b/src/rustc/middle/typeck/check.rs
@@ -893,13 +893,12 @@ fn lookup_op_method(fcx: @fn_ctxt, op_ex: @ast::expr,
                         self_ex: @ast::expr, self_t: ty::t,
                         opname: str, args: ~[@ast::expr])
         -> option<(ty::t, bool)> {
-        let callee_id = ast_util::op_expr_callee_id(op_ex);
         let lkup = method::lookup(fcx, op_ex, self_ex, op_ex.id,
-                                  callee_id, @opname, self_t, ~[], false);
+                     op_ex.callee_id, @opname, self_t, ~[], false);
         alt lkup.method() {
           some(origin) {
             let {fty: method_ty, bot: bot} = {
-                let method_ty = fcx.node_ty(callee_id);
+                let method_ty = fcx.node_ty(op_ex.callee_id);
                 check_call_inner(fcx, op_ex.span, op_ex.id,
                                  method_ty, args)
             };
@@ -1963,7 +1962,9 @@ fn check_constraints(fcx: @fn_ctxt, cs: ~[@ast::constr],
                     }
                     ast::carg_lit(l) {
                       let tmp_node_id = fcx.ccx.tcx.sess.next_node_id();
-                      {id: tmp_node_id, node: ast::expr_lit(l), span: a.span}
+                      {id: tmp_node_id,
+                       callee_id: fcx.ccx.tcx.sess.next_node_id(),
+                       node: ast::expr_lit(l), span: a.span}
                     }
                     ast::carg_ident(i) {
                       if i < num_args {
@@ -1976,6 +1977,7 @@ fn check_constraints(fcx: @fn_ctxt, cs: ~[@ast::constr],
                               (arg_occ_node_id,
                                ast::def_arg(args[i].id, args[i].mode));
                           {id: arg_occ_node_id,
+                           callee_id: fcx.ccx.tcx.sess.next_node_id(),
                            node: ast::expr_path(p),
                            span: a.span}
                       } else {
@@ -1987,11 +1989,14 @@ fn check_constraints(fcx: @fn_ctxt, cs: ~[@ast::constr],
                   });
         }
         let p_op: ast::expr_ = ast::expr_path(c.node.path);
-        let oper: @ast::expr = @{id: c.node.id, node: p_op, span: c.span};
+        let oper: @ast::expr = @{id: c.node.id,
+             callee_id: fcx.ccx.tcx.sess.next_node_id(),
+             node: p_op, span: c.span};
         // Another ephemeral expr
         let call_expr_id = fcx.ccx.tcx.sess.next_node_id();
         let call_expr =
             @{id: call_expr_id,
+              callee_id: fcx.ccx.tcx.sess.next_node_id(),
               node: ast::expr_call(oper, c_args, false),
               span: c.span};
         check_pred_expr(fcx, call_expr);
diff --git a/src/rustc/middle/typeck/check/method.rs b/src/rustc/middle/typeck/check/method.rs
index f1a79fad49b..c3829f60c40 100644
--- a/src/rustc/middle/typeck/check/method.rs
+++ b/src/rustc/middle/typeck/check/method.rs
@@ -415,7 +415,7 @@ fn write_mty_from_candidate(cand: candidate) -> method_map_entry {
         let all_substs = {tps: vec::append(cand.self_substs.tps, m_substs)
                           with cand.self_substs};
 
-        self.fcx.write_ty_substs(self.node_id, cand.fty, all_substs);
+         self.fcx.write_ty_substs(self.node_id, cand.fty, all_substs);
 
         ret cand.entry;
     }
diff --git a/src/rustc/middle/typeck/check/vtable.rs b/src/rustc/middle/typeck/check/vtable.rs
index 42d2529415f..d5401e3a4e7 100644
--- a/src/rustc/middle/typeck/check/vtable.rs
+++ b/src/rustc/middle/typeck/check/vtable.rs
@@ -233,7 +233,7 @@ fn resolve_expr(ex: @ast::expr, &&fcx: @fn_ctxt, v: visit::vt<@fn_ctxt>) {
             if has_trait_bounds(*bounds) {
                 let callee_id = alt ex.node {
                   ast::expr_field(_, _, _) { ex.id }
-                  _ { ast_util::op_expr_callee_id(ex) }
+                  _ { ex.callee_id }
                 };
                 let substs = fcx.node_ty_substs(callee_id);
                 let iscs = cx.impl_map.get(ex.id);
diff --git a/src/rustc/middle/typeck/check/writeback.rs b/src/rustc/middle/typeck/check/writeback.rs
index 9b51a3cb98e..a55c0515d76 100644
--- a/src/rustc/middle/typeck/check/writeback.rs
+++ b/src/rustc/middle/typeck/check/writeback.rs
@@ -105,8 +105,7 @@ fn visit_expr(e: @ast::expr, wbcx: wb_ctxt, v: wb_vt) {
 
       ast::expr_binary(*) | ast::expr_unary(*) | ast::expr_assign_op(*)
         | ast::expr_index(*) {
-        maybe_resolve_type_vars_for_node(wbcx, e.span,
-                                         ast_util::op_expr_callee_id(e));
+        maybe_resolve_type_vars_for_node(wbcx, e.span, e.callee_id);
       }
 
       _ { }
diff --git a/src/test/run-pass/issue-2804-2.rs b/src/test/run-pass/issue-2804-2.rs
new file mode 100644
index 00000000000..245d0d0ab24
--- /dev/null
+++ b/src/test/run-pass/issue-2804-2.rs
@@ -0,0 +1,10 @@
+// Minimized version of issue-2804.rs. Both check that callee IDs don't
+// clobber the previous node ID in a macro expr
+use std;
+import std::map::hashmap;
+
+fn add_interfaces(managed_ip: str, device: std::map::hashmap<str, int>)  {
+     #error["%s, %?", managed_ip, device["interfaces"]];
+}
+
+fn main() {}
diff --git a/src/test/run-pass/issue-2804.rs b/src/test/run-pass/issue-2804.rs
new file mode 100644
index 00000000000..ae63e1ab22e
--- /dev/null
+++ b/src/test/run-pass/issue-2804.rs
@@ -0,0 +1,69 @@
+use std;
+import io;
+import io::writer_util;
+import std::map::hashmap;
+
+enum object
+{
+    bool_value(bool),
+    int_value(i64),
+}
+
+fn lookup(table: std::map::hashmap<str, std::json::json>, key: str, default: str) -> str
+{
+    alt table.find(key)
+    {
+        option::some(std::json::string(s))
+        {
+            *s
+        }
+        option::some(value)
+        {
+            #error["%s was expected to be a string but is a %?", key, value];
+            default
+        }
+        option::none
+        {
+            default
+        }
+    }
+}
+
+fn add_interface(store: int, managed_ip: str, data: std::json::json) -> (str, object)
+{
+    alt data
+    {
+        std::json::dict(interface)
+        {
+            let name = lookup(interface, "ifDescr", "");
+            let label = #fmt["%s-%s", managed_ip, name];
+
+            (label, bool_value(false))
+        }
+        _
+        {
+            #error["Expected dict for %s interfaces but found %?", managed_ip, data];
+            ("gnos:missing-interface", bool_value(true))
+        }
+    }
+}
+
+fn add_interfaces(store: int, managed_ip: str, device: std::map::hashmap<str, std::json::json>) -> [(str, object)]/~
+{
+    alt device["interfaces"]
+    {
+        std::json::list(interfaces)
+        {
+          do vec::map(*interfaces) |interface| {
+                add_interface(store, managed_ip, interface)
+          }
+        }
+        _
+        {
+            #error["Expected list for %s interfaces but found %?", managed_ip, device["interfaces"]];
+            []/~
+        }
+    }
+}
+
+fn main() {}
-- 
GitLab