Fix enum match; disallow comparisons of compound types

crates/analyzer/src/namespace/types.rs

@@ -98,6 +98,7 @@ impl TypeId {
         self.typ(db).has_fixed_size(db)
     }
 
+    // XXX is Contract type usable everywhere Base types are?
     /// `true` if Type::Base or Type::Contract (which is just an Address)
     pub fn is_primitive(&self, db: &dyn AnalyzerDb) -> bool {
         matches!(self.typ(db), Type::Base(_) | Type::Contract(_))

crates/analyzer/src/traversal/call_args.rs

@@ -47,7 +47,8 @@ impl LabeledParameter for (Option<SmolStr>, Result<TypeId, TypeError>) {
     fn typ(&self) -> Result<TypeId, TypeError> {
         self.1.clone()
     }
-    fn is_sink(&self) -> bool { // XXX
+    fn is_sink(&self) -> bool {
+        // XXX
         true
     }
 }

crates/analyzer/src/traversal/expressions.rs

@@ -1616,21 +1616,21 @@ fn expr_call_builtin_value_method(
     };
     match method {
         ValueMethod::ToMem => {
-            if ty.is_base(context.db()) {
-                context.fancy_error(
-                    "`to_mem()` called on primitive type",
-                    vec![
-                        Label::primary(
-                            value.span,
-                            "this value does not need to be explicitly copied to memory",
-                        ),
-                        Label::secondary(method_name.span, "hint: remove `.to_mem()`"),
-                    ],
-                    vec![],
-                );
-            } else if ty.is_sptr(context.db()) {
+            if ty.is_sptr(context.db()) {
                 let inner = ty.deref(context.db());
-                if inner.is_map(context.db()) {
+                if inner.is_primitive(context.db()) {
+                    context.fancy_error(
+                        "`to_mem()` called on primitive type",
+                        vec![
+                            Label::primary(
+                                value.span,
+                                "this value does not need to be explicitly copied to memory",
+                            ),
+                            Label::secondary(method_name.span, "hint: remove `.to_mem()`"),
+                        ],
+                        vec![],
+                    );
+                } else if inner.is_map(context.db()) {
                     context.fancy_error(
                         "`to_mem()` called on a Map",
                         vec![
@@ -1639,18 +1639,21 @@ fn expr_call_builtin_value_method(
                         ],
                         vec![],
                     );
+
+                    // TODO: this restriction should be removed
+                } else if ty.is_generic(context.db()) {
+                    context.fancy_error(
+                        "`to_mem()` called on generic type",
+                        vec![
+                            Label::primary(value.span, "this value can not be copied to memory"),
+                            Label::secondary(method_name.span, "hint: remove `.to_mem()`"),
+                        ],
+                        vec![],
+                    );
                 }
+
                 value_attrs.typ = inner;
                 return Ok((value_attrs, calltype));
-            } else if ty.is_generic(context.db()) {
-                context.fancy_error(
-                    "`to_mem()` called on generic type",
-                    vec![
-                        Label::primary(value.span, "this value can not be copied to memory"),
-                        Label::secondary(method_name.span, "hint: remove `.to_mem()`"),
-                    ],
-                    vec![],
-                );
             } else {
                 context.fancy_error(
                     "`to_mem()` called on value in memory",
@@ -1951,13 +1954,22 @@ fn expr_comp_operation(
     context: &mut dyn AnalyzerContext,
     exp: &Node<fe::Expr>,
 ) -> Result<ExpressionAttributes, FatalError> {
-    if let fe::Expr::CompOperation { left, op: _, right } = &exp.kind {
+    if let fe::Expr::CompOperation { left, op, right } = &exp.kind {
         // comparison operands should be moved to the stack
         let left_ty = value_expr_type(context, left, None)?;
-        expect_expr_type(context, right, left_ty, false)?;
-
-        // XXX test: struct < struct, array != array
-
+        if left_ty.is_primitive(context.db()) {
+            expect_expr_type(context, right, left_ty, false)?;
+        } else {
+            context.error(
+                &format!(
+                    "`{}` type can't be compared with the `{}` operator",
+                    left_ty.display(context.db()),
+                    op.kind
+                ),
+                exp.span,
+                "invalid comparison",
+            );
+        }
         return Ok(ExpressionAttributes::new(TypeId::bool(context.db())));
     }
 

crates/analyzer/src/traversal/functions.rs

@@ -734,7 +734,7 @@ fn func_return(scope: &mut BlockScope, stmt: &Node<fe::FuncStmt>) -> Result<(),
                     &format!(
                         "expected function to return `{}` but was `{}`",
                         expected_type.display(scope.db()),
-                        value_attr.typ.display(scope.db())
+                        value_attr.typ.deref(scope.db()).display(scope.db())
                     ),
                     stmt.span,
                     "",

crates/analyzer/src/traversal/types.rs

@@ -112,7 +112,6 @@ pub fn try_cast_type(
     };
 }
 
-// XXX assert inner is_primitive?
 pub fn deref_type(context: &mut dyn AnalyzerContext, expr: &Node<ast::Expr>, ty: TypeId) -> TypeId {
     match ty.typ(context.db()) {
         Type::SPtr(inner) => adjust_type(context, expr, inner, AdjustmentKind::Load),
@@ -157,6 +156,8 @@ fn coerce(
     should_copy: bool,
     chain: Vec<Adjustment>,
 ) -> Result<Vec<Adjustment>, TypeCoercionError> {
+    // Cut down on some obviously unnecessary copy operations,
+    // because we don't currently optimize MIR.
     let should_copy = should_copy
         && !into.is_sptr(context.db())
         && !into.deref(context.db()).is_primitive(context.db())
@@ -301,9 +302,16 @@ fn add_adjustment_if(
 fn is_temporary(context: &dyn AnalyzerContext, expr: &Node<ast::Expr>) -> bool {
     match &expr.kind {
         ast::Expr::Tuple { .. } | ast::Expr::List { .. } | ast::Expr::Repeat { .. } => true,
+        ast::Expr::Path(path) => {
+            matches!(
+                context.resolve_path(path, expr.span),
+                Ok(NamedThing::EnumVariant(_))
+            )
+        }
         ast::Expr::Call { func, .. } => matches!(
             context.get_call(func),
             Some(CallType::TypeConstructor(_))
+                | Some(CallType::EnumConstructor(_))
                 | Some(CallType::BuiltinValueMethod {
                     method: ValueMethod::ToMem | ValueMethod::AbiEncode,
                     ..

crates/analyzer/tests/errors.rs

@@ -349,3 +349,4 @@ test_file! { uninit_values }
 test_file! { invalid_repeat_length }
 test_file! { invalid_struct_pub_qualifier }
 test_file! { mut_mistakes }
+test_file! { invalid_comparisons }