Detect duplicate record fields in typecheck

3 files changed, 37 insertions, 31 deletions

diff --git a/dhall_syntax/src/core/expr.rs b/dhall_syntax/src/core/expr.rs
index 3bc7504..4bfd224 100644
--- a/dhall_syntax/src/core/expr.rs
+++ b/dhall_syntax/src/core/expr.rs
@@ -1,5 +1,4 @@
 #![allow(non_snake_case)]
-use std::collections::BTreeMap;
 use std::rc::Rc;
 
 use crate::visitor;
@@ -202,13 +201,13 @@ pub enum ExprF<SubExpr, Label, Embed> {
     ///  `Some e`
     SomeLit(SubExpr),
     ///  `{ k1 : t1, k2 : t1 }`
-    RecordType(BTreeMap<Label, SubExpr>),
+    RecordType(Vec<(Label, SubExpr)>),
     ///  `{ k1 = v1, k2 = v2 }`
-    RecordLit(BTreeMap<Label, SubExpr>),
+    RecordLit(Vec<(Label, SubExpr)>),
     ///  `< k1 : t1, k2 >`
-    UnionType(BTreeMap<Label, Option<SubExpr>>),
+    UnionType(Vec<(Label, Option<SubExpr>)>),
     ///  `< k1 = t1, k2 : t2, k3 >`
-    UnionLit(Label, SubExpr, BTreeMap<Label, Option<SubExpr>>),
+    UnionLit(Label, SubExpr, Vec<(Label, Option<SubExpr>)>),
     ///  `merge x y : t`
     Merge(SubExpr, SubExpr, Option<SubExpr>),
     ///  `e.x`
diff --git a/dhall_syntax/src/core/visitor.rs b/dhall_syntax/src/core/visitor.rs
index 20bfc72..1377849 100644
--- a/dhall_syntax/src/core/visitor.rs
+++ b/dhall_syntax/src/core/visitor.rs
@@ -1,5 +1,3 @@
-use std::collections::BTreeMap;
-
 use crate::*;
 
 /// A way too generic Visitor trait.
@@ -68,10 +66,10 @@ where
                 None => None,
             })
         }
-        fn btmap<'a, V, Ret, SE, L, E>(
-            x: &'a BTreeMap<L, SE>,
+        fn vecmap<'a, V, Ret, SE, L, E>(
+            x: &'a Vec<(L, SE)>,
             mut v: V,
-        ) -> Result<BTreeMap<V::L2, V::SE2>, V::Error>
+        ) -> Result<Vec<(V::L2, V::SE2)>, V::Error>
         where
             L: Ord,
             V::L2: Ord,
@@ -81,10 +79,10 @@ where
                 .map(|(k, x)| Ok((v.visit_label(k)?, v.visit_subexpr(x)?)))
                 .collect()
         }
-        fn btoptmap<'a, V, Ret, SE, L, E>(
-            x: &'a BTreeMap<L, Option<SE>>,
+        fn vecoptmap<'a, V, Ret, SE, L, E>(
+            x: &'a Vec<(L, Option<SE>)>,
             mut v: V,
-        ) -> Result<BTreeMap<V::L2, Option<V::SE2>>, V::Error>
+        ) -> Result<Vec<(V::L2, Option<V::SE2>)>, V::Error>
         where
             L: Ord,
             V::L2: Ord,
@@ -147,13 +145,13 @@ where
                 v.visit_subexpr(t)?,
             ),
             SomeLit(e) => SomeLit(v.visit_subexpr(e)?),
-            RecordType(kts) => RecordType(btmap(kts, v)?),
-            RecordLit(kvs) => RecordLit(btmap(kvs, v)?),
-            UnionType(kts) => UnionType(btoptmap(kts, v)?),
+            RecordType(kts) => RecordType(vecmap(kts, v)?),
+            RecordLit(kvs) => RecordLit(vecmap(kvs, v)?),
+            UnionType(kts) => UnionType(vecoptmap(kts, v)?),
             UnionLit(k, x, kts) => UnionLit(
                 v.visit_label(k)?,
                 v.visit_subexpr(x)?,
-                btoptmap(kts, v)?,
+                vecoptmap(kts, v)?,
             ),
             Merge(x, y, t) => Merge(
                 v.visit_subexpr(x)?,
diff --git a/dhall_syntax/src/parser.rs b/dhall_syntax/src/parser.rs
index 607d19c..276510e 100644
--- a/dhall_syntax/src/parser.rs
+++ b/dhall_syntax/src/parser.rs
@@ -2,7 +2,6 @@ use itertools::Itertools;
 use pest::iterators::Pair;
 use pest::Parser;
 use std::borrow::Cow;
-use std::collections::BTreeMap;
 use std::path::PathBuf;
 use std::rc::Rc;
 
@@ -887,29 +886,33 @@ make_parser! {
     ));
 
     rule!(empty_record_literal<ParsedSubExpr> as expression; span;
-        captured_str!(_) => spanned(span, RecordLit(BTreeMap::new()))
+        captured_str!(_) => spanned(span, RecordLit(Vec::new()))
     );
 
     rule!(empty_record_type<ParsedSubExpr> as expression; span;
-        captured_str!(_) => spanned(span, RecordType(BTreeMap::new()))
+        captured_str!(_) => spanned(span, RecordType(Vec::new()))
     );
 
     rule!(non_empty_record_type_or_literal<ParsedSubExpr> as expression; span;
           children!(
         [label(first_label), non_empty_record_type(rest)] => {
             let (first_expr, mut map) = rest;
-            map.insert(first_label, first_expr);
+            map.push((first_label, first_expr));
+            // Sort until we stop using binary decode for parser tests
+            map.sort_by(|(l1, _), (l2, _)| l1.cmp(l2));
             spanned(span, RecordType(map))
         },
         [label(first_label), non_empty_record_literal(rest)] => {
             let (first_expr, mut map) = rest;
-            map.insert(first_label, first_expr);
+            map.push((first_label, first_expr));
+            // Sort until we stop using binary decode for parser tests
+            map.sort_by(|(l1, _), (l2, _)| l1.cmp(l2));
             spanned(span, RecordLit(map))
         },
     ));
 
     rule!(non_empty_record_type
-          <(ParsedSubExpr, BTreeMap<Label, ParsedSubExpr>)>; children!(
+          <(ParsedSubExpr, Vec<(Label, ParsedSubExpr)>)>; children!(
         [expression(expr), record_type_entry(entries)..] => {
             (expr, entries.collect())
         }
@@ -920,7 +923,7 @@ make_parser! {
     ));
 
     rule!(non_empty_record_literal
-          <(ParsedSubExpr, BTreeMap<Label, ParsedSubExpr>)>; children!(
+          <(ParsedSubExpr, Vec<(Label, ParsedSubExpr)>)>; children!(
         [expression(expr), record_literal_entry(entries)..] => {
             (expr, entries.collect())
         }
@@ -932,12 +935,18 @@ make_parser! {
 
     rule!(union_type_or_literal<ParsedSubExpr> as expression; span; children!(
         [empty_union_type(_)] => {
-            spanned(span, UnionType(BTreeMap::new()))
+            spanned(span, UnionType(Vec::new()))
         },
         [non_empty_union_type_or_literal((Some((l, e)), entries))] => {
+            let mut entries = entries;
+            // Sort until we stop using binary decode for parser tests
+            entries.sort_by(|(l1, _), (l2, _)| l1.cmp(l2));
             spanned(span, UnionLit(l, e, entries))
         },
         [non_empty_union_type_or_literal((None, entries))] => {
+            let mut entries = entries;
+            // Sort until we stop using binary decode for parser tests
+            entries.sort_by(|(l1, _), (l2, _)| l1.cmp(l2));
             spanned(span, UnionType(entries))
         },
     ));
@@ -946,20 +955,20 @@ make_parser! {
 
     rule!(non_empty_union_type_or_literal
           <(Option<(Label, ParsedSubExpr)>,
-            BTreeMap<Label, Option<ParsedSubExpr>>)>;
+            Vec<(Label, Option<ParsedSubExpr>)>)>;
             children!(
         [label(l), union_literal_variant_value((e, entries))] => {
             (Some((l, e)), entries)
         },
         [label(l), union_type_or_literal_variant_type((e, rest))] => {
             let (x, mut entries) = rest;
-            entries.insert(l, e);
+            entries.push((l, e));
             (x, entries)
         },
     ));
 
     rule!(union_literal_variant_value
-          <(ParsedSubExpr, BTreeMap<Label, Option<ParsedSubExpr>>)>;
+          <(ParsedSubExpr, Vec<(Label, Option<ParsedSubExpr>)>)>;
             children!(
         [expression(e), union_type_entry(entries)..] => {
             (e, entries.collect())
@@ -975,19 +984,19 @@ make_parser! {
     rule!(union_type_or_literal_variant_type
           <(Option<ParsedSubExpr>,
             (Option<(Label, ParsedSubExpr)>,
-             BTreeMap<Label, Option<ParsedSubExpr>>))>;
+             Vec<(Label, Option<ParsedSubExpr>)>))>;
                 children!(
         [expression(e), non_empty_union_type_or_literal(rest)] => {
             (Some(e), rest)
         },
         [expression(e)] => {
-            (Some(e), (None, BTreeMap::new()))
+            (Some(e), (None, Vec::new()))
         },
         [non_empty_union_type_or_literal(rest)] => {
             (None, rest)
         },
         [] => {
-            (None, (None, BTreeMap::new()))
+            (None, (None, Vec::new()))
         },
     ));