fix(gnovm): improve error message for nil assignment in variable decl…

…aration (gnolang#3068) …aration  <details><summary>Contributors' checklist...</summary> - [ ] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests </details> --------- Co-authored-by: Morgan <[email protected]> Co-authored-by: ltzmaxwell <[email protected]>
gnoswap-labs · Dec 10, 2024 · 72ee3d9 · 72ee3d9
1 parent cac848d
commit 72ee3d9
Show file tree

Hide file tree

Showing 9 changed files with 200 additions and 138 deletions.
diff --git a/gnovm/pkg/gnolang/preprocess.go b/gnovm/pkg/gnolang/preprocess.go
diff --git a/gnovm/pkg/gnolang/type_check.go b/gnovm/pkg/gnolang/type_check.go
@@ -208,8 +208,8 @@ func checkSame(at, bt Type, msg string) error {
 	return nil
 }
 
-func assertAssignableTo(xt, dt Type, autoNative bool) {
-	err := checkAssignableTo(xt, dt, autoNative)
+func assertAssignableTo(n Node, xt, dt Type, autoNative bool) {
+	err := checkAssignableTo(n, xt, dt, autoNative)
 	if err != nil {
 		panic(err.Error())
 	}
@@ -282,7 +282,7 @@ func checkValDefineMismatch(n Node) {
 // Assert that xt can be assigned as dt (dest type).
 // If autoNative is true, a broad range of xt can match against
 // a target native dt type, if and only if dt is a native type.
-func checkAssignableTo(xt, dt Type, autoNative bool) error {
+func checkAssignableTo(n Node, xt, dt Type, autoNative bool) error {
 	if debug {
 		debug.Printf("checkAssignableTo, xt: %v dt: %v \n", xt, dt)
 	}
@@ -292,7 +292,20 @@ func checkAssignableTo(xt, dt Type, autoNative bool) error {
 			return nil
 		}
 		if !maybeNil(dt) {
-			panic(fmt.Sprintf("invalid operation, nil can not be compared to %v", dt))
+			switch n := n.(type) {
+			case *ValueDecl:
+				panic(fmt.Sprintf("cannot use nil as %v value in variable declaration", dt))
+			case *AssignStmt:
+				panic(fmt.Sprintf("cannot use nil as %v value in assignment", dt))
+			case *CompositeLitExpr:
+				panic(fmt.Sprintf("cannot use nil as %v value in array, slice literal or map literal", dt))
+			case *CallExpr:
+				panic(fmt.Sprintf("cannot use nil as %v value in argument to %v", dt, n.Func))
+			case *BinaryExpr:
+				panic(fmt.Sprintf("invalid operation: %v (mismatched types %v and untyped nil)", n, dt))
+			default:
+				panic(fmt.Sprintf("cannot use nil as %v value", dt))
+			}
 		}
 		return nil
 	} else if dt == nil { // _ = xxx, assign8.gno, 0f31. else cases?
@@ -504,7 +517,7 @@ func checkAssignableTo(xt, dt Type, autoNative bool) error {
 		}
 	case *PointerType: // case 4 from here on
 		if pt, ok := xt.(*PointerType); ok {
-			return checkAssignableTo(pt.Elt, cdt.Elt, false)
+			return checkAssignableTo(n, pt.Elt, cdt.Elt, false)
 		}
 	case *ArrayType:
 		if at, ok := xt.(*ArrayType); ok {
@@ -526,7 +539,7 @@ func checkAssignableTo(xt, dt Type, autoNative bool) error {
 	case *SliceType:
 		if st, ok := xt.(*SliceType); ok {
 			if cdt.Vrd {
-				return checkAssignableTo(st.Elt, cdt.Elt, false)
+				return checkAssignableTo(n, st.Elt, cdt.Elt, false)
 			} else {
 				err := checkSame(st.Elt, cdt.Elt, "")
 				if err != nil {
@@ -634,7 +647,7 @@ func (x *BinaryExpr) assertShiftExprCompatible1(store Store, last BlockNode, lt,
 		if lt == UntypedBigdecType {
 			// 1.0 << 1
 			if lic && ric {
-				convertConst(store, last, lcx, UntypedBigintType)
+				convertConst(store, last, x, lcx, UntypedBigintType)
 				return
 			}
 		}
@@ -697,11 +710,11 @@ func (x *BinaryExpr) AssertCompatible(lt, rt Type) {
 		case EQL, NEQ:
 			assertComparable(xt, dt)
 			if !isUntyped(xt) && !isUntyped(dt) {
-				assertAssignableTo(xt, dt, false)
+				assertAssignableTo(x, xt, dt, false)
 			}
 		case LSS, LEQ, GTR, GEQ:
 			if checker, ok := binaryChecker[x.Op]; ok {
-				x.checkCompatibility(xt, dt, checker, x.Op.TokenString())
+				x.checkCompatibility(x, xt, dt, checker, x.Op.TokenString())
 			} else {
 				panic(fmt.Sprintf("checker for %s does not exist", x.Op))
 			}
@@ -710,7 +723,7 @@ func (x *BinaryExpr) AssertCompatible(lt, rt Type) {
 		}
 	} else {
 		if checker, ok := binaryChecker[x.Op]; ok {
-			x.checkCompatibility(xt, dt, checker, x.Op.TokenString())
+			x.checkCompatibility(x, xt, dt, checker, x.Op.TokenString())
 		} else {
 			panic(fmt.Sprintf("checker for %s does not exist", x.Op))
 		}
@@ -738,14 +751,14 @@ func (x *BinaryExpr) AssertCompatible(lt, rt Type) {
 // The function checkOrConvertType will be invoked after this check.
 // NOTE: dt is established based on a specificity check between xt and dt,
 // confirming dt as the appropriate destination type for this context.
-func (x *BinaryExpr) checkCompatibility(xt, dt Type, checker func(t Type) bool, OpStr string) {
+func (x *BinaryExpr) checkCompatibility(n Node, xt, dt Type, checker func(t Type) bool, OpStr string) {
 	if !checker(dt) {
 		panic(fmt.Sprintf("operator %s not defined on: %v", OpStr, kindString(dt)))
 	}
 
 	// if both typed
 	if !isUntyped(xt) && !isUntyped(dt) {
-		err := checkAssignableTo(xt, dt, false)
+		err := checkAssignableTo(n, xt, dt, false)
 		if err != nil {
 			panic(fmt.Sprintf("invalid operation: mismatched types %v and %v", xt, dt))
 		}
@@ -810,19 +823,19 @@ func (x *RangeStmt) AssertCompatible(store Store, last BlockNode) {
 	xt := evalStaticTypeOf(store, last, x.X)
 	switch cxt := xt.(type) {
 	case *MapType:
-		assertAssignableTo(cxt.Key, kt, false)
+		assertAssignableTo(x, cxt.Key, kt, false)
 		if vt != nil {
-			assertAssignableTo(cxt.Value, vt, false)
+			assertAssignableTo(x, cxt.Value, vt, false)
 		}
 	case *SliceType:
 		assertIndexTypeIsInt(kt)
 		if vt != nil {
-			assertAssignableTo(cxt.Elt, vt, false)
+			assertAssignableTo(x, cxt.Elt, vt, false)
 		}
 	case *ArrayType:
 		assertIndexTypeIsInt(kt)
 		if vt != nil {
-			assertAssignableTo(cxt.Elt, vt, false)
+			assertAssignableTo(x, cxt.Elt, vt, false)
 		}
 	case PrimitiveType:
 		if cxt.Kind() == StringKind {
@@ -862,7 +875,7 @@ func (x *AssignStmt) AssertCompatible(store Store, last BlockNode) {
 						assertValidAssignLhs(store, last, lx)
 						if !isBlankIdentifier(lx) {
 							lxt := evalStaticTypeOf(store, last, lx)
-							assertAssignableTo(cft.Results[i].Type, lxt, false)
+							assertAssignableTo(x, cft.Results[i].Type, lxt, false)
 						}
 					}
 				}
@@ -877,7 +890,7 @@ func (x *AssignStmt) AssertCompatible(store Store, last BlockNode) {
 					if !isBlankIdentifier(x.Lhs[0]) { // see composite3.gno
 						dt := evalStaticTypeOf(store, last, x.Lhs[0])
 						ift := evalStaticTypeOf(store, last, cx)
-						assertAssignableTo(ift, dt, false)
+						assertAssignableTo(x, ift, dt, false)
 					}
 					// check second value
 					assertValidAssignLhs(store, last, x.Lhs[1])
@@ -900,12 +913,12 @@ func (x *AssignStmt) AssertCompatible(store Store, last BlockNode) {
 						if _, ok := cx.X.(*NameExpr); ok {
 							rt := evalStaticTypeOf(store, last, cx.X)
 							if mt, ok := rt.(*MapType); ok {
-								assertAssignableTo(mt.Value, lt, false)
+								assertAssignableTo(x, mt.Value, lt, false)
 							}
 						} else if _, ok := cx.X.(*CompositeLitExpr); ok {
 							cpt := evalStaticTypeOf(store, last, cx.X)
 							if mt, ok := cpt.(*MapType); ok {
-								assertAssignableTo(mt.Value, lt, false)
+								assertAssignableTo(x, mt.Value, lt, false)
 							} else {
 								panic("should not happen")
 							}

diff --git a/gnovm/pkg/gnolang/type_check_test.go b/gnovm/pkg/gnolang/type_check_test.go
@@ -51,7 +51,7 @@ func TestCheckAssignableTo(t *testing.T) {
 					}
 				}()
 			}
-			checkAssignableTo(tt.xt, tt.dt, tt.autoNative)
+			checkAssignableTo(nil, tt.xt, tt.dt, tt.autoNative)
 		})
 	}
 }
diff --git a/gnovm/pkg/gnolang/types.go b/gnovm/pkg/gnolang/types.go
@@ -1181,7 +1181,7 @@ func (ft *FuncType) UnboundType(rft FieldType) *FuncType {
 // NOTE: if ft.HasVarg() and !isVarg, argTVs[len(ft.Params):]
 // are ignored (since they are of the same type as
 // argTVs[len(ft.Params)-1]).
-func (ft *FuncType) Specify(store Store, argTVs []TypedValue, isVarg bool) *FuncType {
+func (ft *FuncType) Specify(store Store, n Node, argTVs []TypedValue, isVarg bool) *FuncType {
 	hasGenericParams := false
 	hasGenericResults := false
 	for _, pf := range ft.Params {
@@ -1248,9 +1248,9 @@ func (ft *FuncType) Specify(store Store, argTVs []TypedValue, isVarg bool) *Func
 	for i, pf := range ft.Params {
 		arg := &argTVs[i]
 		if arg.T.Kind() == TypeKind {
-			specifyType(store, lookup, pf.Type, arg.T, arg.GetType())
+			specifyType(store, n, lookup, pf.Type, arg.T, arg.GetType())
 		} else {
-			specifyType(store, lookup, pf.Type, arg.T, nil)
+			specifyType(store, n, lookup, pf.Type, arg.T, nil)
 		}
 	}
 	// apply specifics to generic params and results.
@@ -2427,7 +2427,7 @@ func IsImplementedBy(it Type, ot Type) bool {
 // specTypeval is Type if spec is TypeKind.
 // NOTE: type-checking isn't strictly necessary here, as the resulting lookup
 // map gets applied to produce the ultimate param and result types.
-func specifyType(store Store, lookup map[Name]Type, tmpl Type, spec Type, specTypeval Type) {
+func specifyType(store Store, n Node, lookup map[Name]Type, tmpl Type, spec Type, specTypeval Type) {
 	if isGeneric(spec) {
 		panic("spec must not be generic")
 	}
@@ -2441,11 +2441,11 @@ func specifyType(store Store, lookup map[Name]Type, tmpl Type, spec Type, specTy
 	case *PointerType:
 		switch pt := baseOf(spec).(type) {
 		case *PointerType:
-			specifyType(store, lookup, ct.Elt, pt.Elt, nil)
+			specifyType(store, n, lookup, ct.Elt, pt.Elt, nil)
 		case *NativeType:
 			// NOTE: see note about type-checking.
 			et := pt.Elem()
-			specifyType(store, lookup, ct.Elt, et, nil)
+			specifyType(store, n, lookup, ct.Elt, et, nil)
 		default:
 			panic(fmt.Sprintf(
 				"expected pointer kind but got %s",
@@ -2454,11 +2454,11 @@ func specifyType(store Store, lookup map[Name]Type, tmpl Type, spec Type, specTy
 	case *ArrayType:
 		switch at := baseOf(spec).(type) {
 		case *ArrayType:
-			specifyType(store, lookup, ct.Elt, at.Elt, nil)
+			specifyType(store, n, lookup, ct.Elt, at.Elt, nil)
 		case *NativeType:
 			// NOTE: see note about type-checking.
 			et := at.Elem()
-			specifyType(store, lookup, ct.Elt, et, nil)
+			specifyType(store, n, lookup, ct.Elt, et, nil)
 		default:
 			panic(fmt.Sprintf(
 				"expected array kind but got %s",
@@ -2469,7 +2469,7 @@ func specifyType(store Store, lookup map[Name]Type, tmpl Type, spec Type, specTy
 		case PrimitiveType:
 			if isGeneric(ct.Elt) {
 				if st.Kind() == StringKind {
-					specifyType(store, lookup, ct.Elt, Uint8Type, nil)
+					specifyType(store, n, lookup, ct.Elt, Uint8Type, nil)
 				} else {
 					panic(fmt.Sprintf(
 						"expected slice kind but got %s",
@@ -2485,11 +2485,11 @@ func specifyType(store Store, lookup map[Name]Type, tmpl Type, spec Type, specTy
 					spec.Kind()))
 			}
 		case *SliceType:
-			specifyType(store, lookup, ct.Elt, st.Elt, nil)
+			specifyType(store, n, lookup, ct.Elt, st.Elt, nil)
 		case *NativeType:
 			// NOTE: see note about type-checking.
 			et := st.Elem()
-			specifyType(store, lookup, ct.Elt, et, nil)
+			specifyType(store, n, lookup, ct.Elt, et, nil)
 		default:
 			panic(fmt.Sprintf(
 				"expected slice kind but got %s",
@@ -2498,14 +2498,14 @@ func specifyType(store Store, lookup map[Name]Type, tmpl Type, spec Type, specTy
 	case *MapType:
 		switch mt := baseOf(spec).(type) {
 		case *MapType:
-			specifyType(store, lookup, ct.Key, mt.Key, nil)
-			specifyType(store, lookup, ct.Value, mt.Value, nil)
+			specifyType(store, n, lookup, ct.Key, mt.Key, nil)
+			specifyType(store, n, lookup, ct.Value, mt.Value, nil)
 		case *NativeType:
 			// NOTE: see note about type-checking.
 			kt := mt.Key()
 			vt := mt.Elem()
-			specifyType(store, lookup, ct.Key, kt, nil)
-			specifyType(store, lookup, ct.Value, vt, nil)
+			specifyType(store, n, lookup, ct.Key, kt, nil)
+			specifyType(store, n, lookup, ct.Value, vt, nil)
 		default:
 			panic(fmt.Sprintf(
 				"expected map kind but got %s",
@@ -2544,7 +2544,7 @@ func specifyType(store Store, lookup map[Name]Type, tmpl Type, spec Type, specTy
 				generic := ct.Generic[:len(ct.Generic)-len(".Elem()")]
 				match, ok := lookup[generic]
 				if ok {
-					assertAssignableTo(spec, match.Elem(), false)
+					assertAssignableTo(n, spec, match.Elem(), false)
 					return // ok
 				} else {
 					// Panic here, because we don't know whether T
@@ -2558,7 +2558,7 @@ func specifyType(store Store, lookup map[Name]Type, tmpl Type, spec Type, specTy
 			} else {
 				match, ok := lookup[ct.Generic]
 				if ok {
-					assertAssignableTo(spec, match, false)
+					assertAssignableTo(n, spec, match, false)
 					return // ok
 				} else {
 					if isUntyped(spec) {
@@ -2576,9 +2576,9 @@ func specifyType(store Store, lookup map[Name]Type, tmpl Type, spec Type, specTy
 		switch cbt := baseOf(spec).(type) {
 		case *NativeType:
 			gnoType := store.Go2GnoType(cbt.Type)
-			specifyType(store, lookup, ct.Type, gnoType, nil)
+			specifyType(store, n, lookup, ct.Type, gnoType, nil)
 		default:
-			specifyType(store, lookup, ct.Type, cbt, nil)
+			specifyType(store, n, lookup, ct.Type, cbt, nil)
 		}
 	default:
 		// ignore, no generics.

diff --git a/gnovm/tests/files/add3.gno b/gnovm/tests/files/add3.gno
@@ -0,0 +1,9 @@
+package main
+
+func main() {
+	a := 1
+	i := a + nil
+}
+
+// Error:
+// main/files/add3.gno:5:7: invalid operation: a<VPBlock(1,0)> + (const (undefined)) (mismatched types int and untyped nil)
diff --git a/gnovm/tests/files/assign38.gno b/gnovm/tests/files/assign38.gno
@@ -0,0 +1,10 @@
+package main
+
+func main() {
+	a := 1
+	a = nil
+	println(a)
+}
+
+// Error:
+// main/files/assign38.gno:5:2: cannot use nil as int value in assignment
diff --git a/gnovm/tests/files/fun28.gno b/gnovm/tests/files/fun28.gno
@@ -0,0 +1,10 @@
+package main
+
+func f(i int) {}
+
+func main() {
+	f(nil)
+}
+
+// Error:
+// main/files/fun28.gno:6:2: cannot use nil as int value in argument to f<VPBlock(3,0)>
diff --git a/gnovm/tests/files/slice3.gno b/gnovm/tests/files/slice3.gno
@@ -0,0 +1,9 @@
+package main
+
+func main() {
+	i := []string{nil}
+	println(i)
+}
+
+// Error:
+// main/files/slice3.gno:4:7: cannot use nil as string value in array, slice literal or map literal
diff --git a/gnovm/tests/files/var35.gno b/gnovm/tests/files/var35.gno
@@ -0,0 +1,8 @@
+package main
+
+func main() {
+	var i int = nil
+}
+
+// Error:
+// main/files/var35.gno:4:6: cannot use nil as int value in variable declaration
-Original file line number
+Diff line change
@@ Expand Up / @@ -51,7 +51,7 @@ func TestCheckAssignableTo(t *testing.T) { @@
     					}
     				}()
     			}
-    			checkAssignableTo(tt.xt, tt.dt, tt.autoNative)
+    			checkAssignableTo(nil, tt.xt, tt.dt, tt.autoNative)
     		})
     	}
     }