From f7382c454fb92f6c28ac6198821304c0ae2a080a Mon Sep 17 00:00:00 2001
From: Tim Chevalier <chevalier@alum.wellesley.edu>
Date: Fri, 27 Jul 2012 15:38:01 -0700
Subject: [PATCH] Forbid duplicate fields in record types and exprs

Closes #3033
---
 src/rustc/middle/resolve3.rs        |  2 +-
 src/rustc/middle/typeck/check.rs    | 38 +++++++++++++++++++++++++++++
 src/test/compile-fail/issue-3033.rs |  6 +++++
 3 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 src/test/compile-fail/issue-3033.rs

diff --git a/src/rustc/middle/resolve3.rs b/src/rustc/middle/resolve3.rs
index 65c8f4b4dd6..8af823ac072 100644
--- a/src/rustc/middle/resolve3.rs
+++ b/src/rustc/middle/resolve3.rs
@@ -47,7 +47,7 @@ import str::{connect, split_str};
 import vec::pop;
 
 import std::list::{cons, list, nil};
-import std::map::{hashmap, int_hash, str_hash};
+import std::map::{hashmap, int_hash};
 import ASTMap = syntax::ast_map::map;
 import str_eq = str::eq;
 
diff --git a/src/rustc/middle/typeck/check.rs b/src/rustc/middle/typeck/check.rs
index 9e95910358f..ce7536b5bf9 100644
--- a/src/rustc/middle/typeck/check.rs
+++ b/src/rustc/middle/typeck/check.rs
@@ -376,6 +376,29 @@ fn check_class_member(ccx: @crate_ctxt, class_t: ty::t,
     }
 }
 
+fn check_no_duplicate_fields(tcx: ty::ctxt, fields:
+                             ~[(ast::ident, span)]) {
+    let field_names = hashmap::<@~str, span>(|x| str::hash(*x),
+                                             |x,y| str::eq(*x, *y));
+    for fields.each |p| {
+        let (id, sp) = p;
+        alt field_names.find(id) {
+          some(orig_sp) {
+            tcx.sess.span_err(sp, #fmt("Duplicate field \
+                                   name %s in record type declaration",
+                                   *id));
+            tcx.sess.span_note(orig_sp, ~"First declaration of \
+                                          this field occurred here");
+            break;
+          }
+          none {
+            field_names.insert(id, sp);
+          }
+        }
+    }
+
+}
+
 fn check_item(ccx: @crate_ctxt, it: @ast::item) {
     alt it.node {
       ast::item_const(_, e) { check_const(ccx, it.span, e, it.id); }
@@ -429,6 +452,14 @@ fn check_item(ccx: @crate_ctxt, it: @ast::item) {
       ast::item_ty(t, tps) {
         let tpt_ty = ty::node_id_to_type(ccx.tcx, it.id);
         check_bounds_are_used(ccx, t.span, tps, tpt_ty);
+        // If this is a record ty, check for duplicate fields
+        alt t.node {
+            ast::ty_rec(fields) {
+              check_no_duplicate_fields(ccx.tcx, fields.map(|f|
+                                              (f.node.ident, f.span)));
+            }
+            _ {}
+        }
       }
       ast::item_foreign_mod(m) {
         if syntax::attr::foreign_abi(it.attrs) ==
@@ -1617,6 +1648,13 @@ fn check_expr_with_unifier(fcx: @fn_ctxt,
             fn get_node(f: spanned<field>) -> field { f.node }
             let typ = ty::mk_rec(tcx, vec::map(fields_t, get_node));
             fcx.write_ty(id, typ);
+            /* Check for duplicate fields */
+            /* Only do this check if there's no base expr -- the reason is
+               that we're extending a record we know has no dup fields, and
+               it would be ill-typed anyway if we duplicated one of its
+               fields */
+            check_no_duplicate_fields(tcx, fields.map(|f|
+                                                    (f.node.ident, f.span)));
           }
           some(bexpr) {
             let bexpr_t = fcx.expr_ty(bexpr);
diff --git a/src/test/compile-fail/issue-3033.rs b/src/test/compile-fail/issue-3033.rs
new file mode 100644
index 00000000000..f86c22bc067
--- /dev/null
+++ b/src/test/compile-fail/issue-3033.rs
@@ -0,0 +1,6 @@
+type cat = {cat_name: ~str, cat_name: int};  //~ ERROR Duplicate field name cat_name
+
+fn main()
+{
+  io::println(int::str({x: 1, x: 2}.x)); //~ ERROR Duplicate field name x
+}