Fix issue with generic parameters that are marked mut (#865)

ethereum · Apr 8, 2023 · 44b2f54 · 44b2f54
1 parent 2dcdaaa
commit 44b2f54
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 57 deletions.
diff --git a/crates/analyzer/src/namespace/types.rs b/crates/analyzer/src/namespace/types.rs
@@ -100,6 +100,9 @@ impl TypeId {
     pub fn typ(&self, db: &dyn AnalyzerDb) -> Type {
         db.lookup_intern_type(*self)
     }
+    pub fn deref_typ(&self, db: &dyn AnalyzerDb) -> Type {
+        self.deref(db).typ(db)
+    }
     pub fn deref(self, db: &dyn AnalyzerDb) -> TypeId {
         match self.typ(db) {
             Type::SPtr(inner) => inner,

diff --git a/crates/analyzer/src/traversal/call_args.rs b/crates/analyzer/src/traversal/call_args.rs
@@ -163,56 +163,57 @@ pub fn validate_named_args(
 
         let param_type = param.typ()?;
         // Check arg type
-        let arg_type = if let Type::Generic(Generic { bounds, .. }) = param_type.typ(context.db()) {
-            let arg_type = expr_type(context, &arg.kind.value)?;
-            for bound in bounds.iter() {
-                if !bound.is_implemented_for(context.db(), arg_type) {
-                    context.error(
-                        &format!(
-                            "the trait bound `{}: {}` is not satisfied",
-                            arg_type.display(context.db()),
-                            bound.name(context.db())
-                        ),
-                        arg.span,
-                        &format!(
-                            "the trait `{}` is not implemented for `{}`",
-                            bound.name(context.db()),
-                            arg_type.display(context.db()),
-                        ),
-                    );
-                }
-            }
-            arg_type
-        } else {
-            let arg_attr = expr(context, &arg.kind.value, Some(param_type))?;
-            match try_coerce_type(
-                context,
-                Some(&arg.kind.value),
-                arg_attr.typ,
-                param_type,
-                param.is_sink(),
-            ) {
-                Err(TypeCoercionError::Incompatible) => {
-                    let msg = if let Some(label) = param.label() {
-                        format!("incorrect type for `{name}` argument `{label}`")
-                    } else {
-                        format!("incorrect type for `{name}` argument at position {index}")
-                    };
-                    context.type_error(&msg, arg.kind.value.span, param_type, arg_attr.typ);
-                }
-                Err(TypeCoercionError::RequiresToMem) => {
-                    context.add_diagnostic(errors::to_mem_error(arg.span));
+        let arg_type =
+            if let Type::Generic(Generic { bounds, .. }) = param_type.deref_typ(context.db()) {
+                let arg_type = expr_type(context, &arg.kind.value)?;
+                for bound in bounds.iter() {
+                    if !bound.is_implemented_for(context.db(), arg_type) {
+                        context.error(
+                            &format!(
+                                "the trait bound `{}: {}` is not satisfied",
+                                arg_type.display(context.db()),
+                                bound.name(context.db())
+                            ),
+                            arg.span,
+                            &format!(
+                                "the trait `{}` is not implemented for `{}`",
+                                bound.name(context.db()),
+                                arg_type.display(context.db()),
+                            ),
+                        );
+                    }
                 }
-                Err(TypeCoercionError::SelfContractType) => {
-                    context.add_diagnostic(errors::self_contract_type_error(
-                        arg.span,
-                        &param_type.display(context.db()),
-                    ));
+                arg_type
+            } else {
+                let arg_attr = expr(context, &arg.kind.value, Some(param_type))?;
+                match try_coerce_type(
+                    context,
+                    Some(&arg.kind.value),
+                    arg_attr.typ,
+                    param_type,
+                    param.is_sink(),
+                ) {
+                    Err(TypeCoercionError::Incompatible) => {
+                        let msg = if let Some(label) = param.label() {
+                            format!("incorrect type for `{name}` argument `{label}`")
+                        } else {
+                            format!("incorrect type for `{name}` argument at position {index}")
+                        };
+                        context.type_error(&msg, arg.kind.value.span, param_type, arg_attr.typ);
+                    }
+                    Err(TypeCoercionError::RequiresToMem) => {
+                        context.add_diagnostic(errors::to_mem_error(arg.span));
+                    }
+                    Err(TypeCoercionError::SelfContractType) => {
+                        context.add_diagnostic(errors::self_contract_type_error(
+                            arg.span,
+                            &param_type.display(context.db()),
+                        ));
+                    }
+                    Ok(_) => {}
                 }
-                Ok(_) => {}
-            }
-            arg_attr.typ
-        };
+                arg_attr.typ
+            };
 
         if param_type.is_mut(context.db()) && !arg_type.is_mut(context.db()) {
             let msg = if let Some(label) = param.label() {

diff --git a/crates/mir/src/lower/function.rs b/crates/mir/src/lower/function.rs
@@ -53,12 +53,12 @@ pub fn lower_monomorphized_func_signature(
     for param in analyzer_signature.params.iter() {
         let source = arg_source(db, func, &param.name);
 
-        let param_type = if let Type::Generic(generic) = param.typ.clone().unwrap().typ(db.upcast())
-        {
-            *resolved_generics.get(&generic.name).unwrap()
-        } else {
-            param.typ.clone().unwrap()
-        };
+        let param_type =
+            if let Type::Generic(generic) = param.typ.clone().unwrap().deref_typ(db.upcast()) {
+                *resolved_generics.get(&generic.name).unwrap()
+            } else {
+                param.typ.clone().unwrap()
+            };
 
         params.push(make_param(db, param.clone().name, param_type, source))
     }
@@ -588,7 +588,7 @@ impl<'db, 'a> BodyLowerHelper<'db, 'a> {
     fn lower_analyzer_type(&self, analyzer_ty: analyzer_types::TypeId) -> TypeId {
         // If the analyzer type is generic we first need to resolve it to its concrete
         // type before lowering to a MIR type
-        if let analyzer_types::Type::Generic(generic) = analyzer_ty.typ(self.db.upcast()) {
+        if let analyzer_types::Type::Generic(generic) = analyzer_ty.deref_typ(self.db.upcast()) {
             let resolved_type = self
                 .func
                 .signature(self.db)
@@ -925,7 +925,9 @@ impl<'db, 'a> BodyLowerHelper<'db, 'a> {
                     .expect("invalid parameter")
             }))
             .filter_map(|(param, typ)| {
-                if let Type::Generic(generic) = param.typ.clone().unwrap().typ(self.db.upcast()) {
+                if let Type::Generic(generic) =
+                    param.typ.clone().unwrap().deref_typ(self.db.upcast())
+                {
                     Some((generic.name, typ))
                 } else {
                     None

diff --git a/crates/test-files/fixtures/features/generic_functions.fe b/crates/test-files/fixtures/features/generic_functions.fe
@@ -36,10 +36,17 @@ struct Runner {
     return val.compute(val: 1000)
   }
 
+  pub fn run_mut<T: Computable>(self, mut _ val: T) -> u256 {
+    return val.compute(val: 1000)
+  }
+
   pub fn run_static<T: Computable>(_ val: T) -> u256 {
     return val.compute(val: 1000)
   }
 
+  pub fn run_mut_static<T: Computable>(mut _ val: T) -> u256 {
+    return val.compute(val: 1000)
+  }
 
   pub fn dummy<T: Dummy>(self, _ val: T) {
 
@@ -52,8 +59,16 @@ contract Example {
     assert runner.run(Mac()) == 1001
     assert runner.run(Linux(counter: 10)) == 1015
 
+    let mut linux: Linux = Linux(counter: 10);
+    let mut mac: Mac = Mac();
+
+    assert runner.run_mut(mac) == 1001
+    assert runner.run_mut(linux) == 1015
+
     assert Runner::run_static(Mac()) == 1001
 
+    assert Runner::run_mut_static(mac) == 1001
+
     // We are testing that we can satisfy a trait bound where the trait is defined in another module
     runner.dummy(Mac())
   }

diff --git a/...c/snapshots/fe_compiler_tests_legacy__features__execution_tests@generic_functions.fe.snap b/...c/snapshots/fe_compiler_tests_legacy__features__execution_tests@generic_functions.fe.snap
@@ -1,6 +1,7 @@
 ---
 source: crates/tests-legacy/src/features.rs
+assertion_line: 2231
 expression: "format!(\"{}\", harness.gas_reporter)"
 ---
-run_test([]) used 32 gas
+run_test([]) used 459 gas
 
diff --git a/newsfragments/865.feature.md b/newsfragments/865.feature.md
@@ -0,0 +1,20 @@
+Fixed an issue where generic parameters that were `mut` could not be satisfied at callsite.
+
+For instance, the following code would previously cause a compile error but now works as expected:
+
+```rust
+struct Runner {
+  pub fn run<T: Computable>(self, mut _ val: T) -> u256 {
+    return val.compute(val: 1000)
+  }
+}
+
+contract Example {
+  pub fn run_test(self) {
+    let runner: Runner = Runner();
+    let mut mac: Mac = Mac();
+
+    assert runner.run(mac) == 1001
+  }
+}
+```