Merge pull request #384 from sbillig/generic-map

Remove ast::TypeDesc::Map in favor of TypeDesc::Generic
ethereum · May 3, 2021 · be3ccf8 · be3ccf8
2 parents 5c25397 + 9fb10f8
commit be3ccf8
Show file tree

Hide file tree

Showing 20 changed files with 369 additions and 237 deletions.
diff --git a/analyzer/src/errors.rs b/analyzer/src/errors.rs
@@ -11,6 +11,7 @@ pub enum ErrorKind {
     CannotMove,
     CircularDependency,
     ContinueWithoutLoop,
+    MapTypeError,
     KeyWordArgsRequired,
     MissingEventDefinition,
     MissingReturn,

diff --git a/analyzer/src/namespace/types.rs b/analyzer/src/namespace/types.rs
@@ -1,5 +1,6 @@
-use crate::errors::SemanticError;
+use crate::errors::{ErrorKind, SemanticError};
 use fe_parser::ast as fe;
+use fe_parser::node::Node;
 use std::collections::{btree_map::Entry, BTreeMap, HashMap};
 use std::convert::TryFrom;
 
@@ -918,11 +919,28 @@ pub fn type_desc(defs: &HashMap<String, Type>, typ: &fe::TypeDesc) -> Result<Typ
             inner: type_desc_base(defs, &typ.kind)?,
             size: *dimension,
         })),
-        fe::TypeDesc::Map { from, to } => Ok(Type::Map(Map {
-            key: type_desc_base(defs, &from.kind)?,
-            value: Box::new(type_desc(defs, &to.kind)?),
-        })),
-        fe::TypeDesc::Generic { .. } => todo!(),
+        fe::TypeDesc::Generic { base, args } => {
+            if base.kind == "map" {
+                match &args[..] {
+                    [Node {
+                        kind: fe::GenericArg::TypeDesc(from),
+                        ..
+                    }, Node {
+                        kind: fe::GenericArg::TypeDesc(to),
+                        ..
+                    }] => Ok(Type::Map(Map {
+                        key: type_desc_base(defs, &from)?,
+                        value: Box::new(type_desc(defs, &to)?),
+                    })),
+                    _ => Err(SemanticError {
+                        kind: ErrorKind::MapTypeError,
+                        context: vec![],
+                    }),
+                }
+            } else {
+                Err(SemanticError::undefined_value())
+            }
+        }
         fe::TypeDesc::Tuple { items } => Ok(Type::Tuple(Tuple {
             items: items
                 .iter()

diff --git a/compiler/tests/cases/compile_errors.rs b/compiler/tests/cases/compile_errors.rs
@@ -10,6 +10,11 @@ use rstest::rstest;
     fixture_file,
     expected_error,
     case("assert_reason_not_string.fe", TypeError),
+    case("bad_map.fe", MapTypeError),
+    case("bad_map2.fe", MapTypeError),
+    case("bad_map3.fe", MapTypeError),
+    case("bad_map4.fe", MapTypeError),
+    case("bad_map5.fe", TypeError),
     case("break_without_loop_2.fe", BreakWithoutLoop),
     case("break_without_loop.fe", BreakWithoutLoop),
     case("call_event_with_wrong_types.fe", TypeError),

diff --git a/compiler/tests/fixtures/compile_errors/bad_map.fe b/compiler/tests/fixtures/compile_errors/bad_map.fe
@@ -0,0 +1,2 @@
+contract Foo:
+    x: map<u8, u8, u8>
diff --git a/compiler/tests/fixtures/compile_errors/bad_map2.fe b/compiler/tests/fixtures/compile_errors/bad_map2.fe
@@ -0,0 +1,2 @@
+contract Foo:
+  x: map<address, 100>
diff --git a/compiler/tests/fixtures/compile_errors/bad_map3.fe b/compiler/tests/fixtures/compile_errors/bad_map3.fe
@@ -0,0 +1,2 @@
+contract Foo:
+    x: map<>
diff --git a/compiler/tests/fixtures/compile_errors/bad_map4.fe b/compiler/tests/fixtures/compile_errors/bad_map4.fe
@@ -0,0 +1,2 @@
+contract Foo:
+    x: map<y>
diff --git a/compiler/tests/fixtures/compile_errors/bad_map5.fe b/compiler/tests/fixtures/compile_errors/bad_map5.fe
@@ -0,0 +1,2 @@
+contract Foo:
+  x: map<map<u8, u8>, address>
diff --git a/parser/src/ast.rs b/parser/src/ast.rs
@@ -40,10 +40,6 @@ pub enum TypeDesc {
         typ: Box<Node<TypeDesc>>,
         dimension: usize,
     },
-    Map {
-        from: Box<Node<TypeDesc>>,
-        to: Box<Node<TypeDesc>>,
-    },
     Tuple {
         items: Vec<Node<TypeDesc>>,
     },

diff --git a/parser/src/grammar/types.rs b/parser/src/grammar/types.rs
@@ -261,26 +261,6 @@ fn parse_generic_args(par: &mut Parser) -> ParseResult<(Vec<Node<GenericArg>>, S
     Ok((args, span))
 }
 
-// TODO: remove TypeDesc::Map and this fn
-fn temporary_backward_compatible_map_type(
-    mut args: Vec<Node<GenericArg>>,
-    span: Span,
-) -> Node<TypeDesc> {
-    let to_node = args.pop().unwrap();
-    let from_node = args.pop().unwrap();
-    if let (GenericArg::TypeDesc(from), GenericArg::TypeDesc(to)) = (from_node.kind, to_node.kind) {
-        Node::new(
-            TypeDesc::Map {
-                from: Box::new(Node::new(from, from_node.span)),
-                to: Box::new(Node::new(to, to_node.span)),
-            },
-            span,
-        )
-    } else {
-        panic!()
-    }
-}
-
 /// Parse a type description, e.g. `u8` or `map<address, u256>`.
 pub fn parse_type_desc(par: &mut Parser) -> ParseResult<Node<TypeDesc>> {
     use TokenKind::*;
@@ -292,17 +272,13 @@ pub fn parse_type_desc(par: &mut Parser) -> ParseResult<Node<TypeDesc>> {
                 Some(Lt) => {
                     let (args, argspan) = parse_generic_args(par)?;
                     let span = name.span + argspan;
-                    if name.text == "map" {
-                        temporary_backward_compatible_map_type(args, span)
-                    } else {
-                        Node::new(
-                            TypeDesc::Generic {
-                                base: name.into(),
-                                args,
-                            },
-                            span,
-                        )
-                    }
+                    Node::new(
+                        TypeDesc::Generic {
+                            base: name.into(),
+                            args,
+                        },
+                        span,
+                    )
                 }
                 _ => Node::new(
                     TypeDesc::Base {

diff --git a/parser/tests/cases/snapshots/cases__parse_ast__contract_def.snap b/parser/tests/cases/snapshots/cases__parse_ast__contract_def.snap
@@ -97,25 +97,34 @@ Node(
             ),
           ),
           typ: Node(
-            kind: Map(
-              from: Node(
-                kind: Base(
-                  base: "u8",
-                ),
+            kind: Generic(
+              base: Node(
+                kind: "map",
                 span: Span(
-                  start: 58,
-                  end: 60,
+                  start: 54,
+                  end: 57,
                 ),
               ),
-              to: Node(
-                kind: Base(
-                  base: "address",
+              args: [
+                Node(
+                  kind: TypeDesc(Base(
+                    base: "u8",
+                  )),
+                  span: Span(
+                    start: 58,
+                    end: 60,
+                  ),
                 ),
-                span: Span(
-                  start: 62,
-                  end: 69,
+                Node(
+                  kind: TypeDesc(Base(
+                    base: "address",
+                  )),
+                  span: Span(
+                    start: 62,
+                    end: 69,
+                  ),
                 ),
-              ),
+              ],
             ),
             span: Span(
               start: 54,

diff --git a/parser/tests/cases/snapshots/cases__parse_ast__guest_book.snap b/parser/tests/cases/snapshots/cases__parse_ast__guest_book.snap
@@ -67,25 +67,34 @@ Node(
                   ),
                 ),
                 typ: Node(
-                  kind: Map(
-                    from: Node(
-                      kind: Base(
-                        base: "address",
-                      ),
+                  kind: Generic(
+                    base: Node(
+                      kind: "map",
                       span: Span(
-                        start: 72,
-                        end: 79,
+                        start: 68,
+                        end: 71,
                       ),
                     ),
-                    to: Node(
-                      kind: Base(
-                        base: "BookMsg",
+                    args: [
+                      Node(
+                        kind: TypeDesc(Base(
+                          base: "address",
+                        )),
+                        span: Span(
+                          start: 72,
+                          end: 79,
+                        ),
                       ),
-                      span: Span(
-                        start: 81,
-                        end: 88,
+                      Node(
+                        kind: TypeDesc(Base(
+                          base: "BookMsg",
+                        )),
+                        span: Span(
+                          start: 81,
+                          end: 88,
+                        ),
                       ),
-                    ),
+                    ],
                   ),
                   span: Span(
                     start: 68,

diff --git a/parser/tests/cases/snapshots/cases__parse_ast__module_stmts.snap b/parser/tests/cases/snapshots/cases__parse_ast__module_stmts.snap
@@ -74,25 +74,34 @@ Node(
             ),
           ),
           typ: Node(
-            kind: Map(
-              from: Node(
-                kind: Base(
-                  base: "u8",
-                ),
+            kind: Generic(
+              base: Node(
+                kind: "map",
                 span: Span(
-                  start: 45,
-                  end: 47,
+                  start: 41,
+                  end: 44,
                 ),
               ),
-              to: Node(
-                kind: Base(
-                  base: "u16",
+              args: [
+                Node(
+                  kind: TypeDesc(Base(
+                    base: "u8",
+                  )),
+                  span: Span(
+                    start: 45,
+                    end: 47,
+                  ),
                 ),
-                span: Span(
-                  start: 49,
-                  end: 52,
+                Node(
+                  kind: TypeDesc(Base(
+                    base: "u16",
+                  )),
+                  span: Span(
+                    start: 49,
+                    end: 52,
+                  ),
                 ),
-              ),
+              ],
             ),
             span: Span(
               start: 41,

diff --git a/parser/tests/cases/snapshots/cases__parse_ast__struct_def.snap b/parser/tests/cases/snapshots/cases__parse_ast__struct_def.snap
@@ -130,25 +130,34 @@ Node(
             ),
           ),
           typ: Node(
-            kind: Map(
-              from: Node(
-                kind: Base(
-                  base: "u8",
-                ),
+            kind: Generic(
+              base: Node(
+                kind: "map",
                 span: Span(
-                  start: 68,
-                  end: 70,
+                  start: 64,
+                  end: 67,
                 ),
               ),
-              to: Node(
-                kind: Base(
-                  base: "foo",
+              args: [
+                Node(
+                  kind: TypeDesc(Base(
+                    base: "u8",
+                  )),
+                  span: Span(
+                    start: 68,
+                    end: 70,
+                  ),
                 ),
-                span: Span(
-                  start: 72,
-                  end: 75,
+                Node(
+                  kind: TypeDesc(Base(
+                    base: "foo",
+                  )),
+                  span: Span(
+                    start: 72,
+                    end: 75,
+                  ),
                 ),
-              ),
+              ],
             ),
             span: Span(
               start: 64,

diff --git a/parser/tests/cases/snapshots/cases__parse_ast__type_def.snap b/parser/tests/cases/snapshots/cases__parse_ast__type_def.snap
@@ -13,25 +13,34 @@ Node(
       ),
     ),
     typ: Node(
-      kind: Map(
-        from: Node(
-          kind: Base(
-            base: "address",
-          ),
+      kind: Generic(
+        base: Node(
+          kind: "map",
           span: Span(
-            start: 13,
-            end: 20,
+            start: 9,
+            end: 12,
           ),
         ),
-        to: Node(
-          kind: Base(
-            base: "u256",
+        args: [
+          Node(
+            kind: TypeDesc(Base(
+              base: "address",
+            )),
+            span: Span(
+              start: 13,
+              end: 20,
+            ),
           ),
-          span: Span(
-            start: 22,
-            end: 26,
+          Node(
+            kind: TypeDesc(Base(
+              base: "u256",
+            )),
+            span: Span(
+              start: 22,
+              end: 26,
+            ),
           ),
-        ),
+        ],
       ),
       span: Span(
         start: 9,