Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gnovm): improve error message for nil assignment in variable declaration #3068

Merged
merged 20 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 101 additions & 98 deletions gnovm/pkg/gnolang/preprocess.go

Large diffs are not rendered by default.

53 changes: 33 additions & 20 deletions gnovm/pkg/gnolang/type_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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)
ltzmaxwell marked this conversation as resolved.
Show resolved Hide resolved
}
Expand All @@ -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?
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}
}
Expand All @@ -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])
Expand All @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion gnovm/pkg/gnolang/type_check_test.go
Original file line number Diff 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)
})
}
}
38 changes: 19 additions & 19 deletions gnovm/pkg/gnolang/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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")
}
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions gnovm/tests/files/add3.gno
Original file line number Diff line number Diff line change
@@ -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)
10 changes: 10 additions & 0 deletions gnovm/tests/files/assign38.gno
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions gnovm/tests/files/fun28.gno
Original file line number Diff line number Diff line change
@@ -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)>
9 changes: 9 additions & 0 deletions gnovm/tests/files/slice3.gno
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions gnovm/tests/files/var35.gno
Original file line number Diff line number Diff line change
@@ -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
Loading