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 15 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
191 changes: 97 additions & 94 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 @@
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 @@
// 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 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
}
} 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 @@
// 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 @@
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)

Check warning on line 828 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L828

Added line #L828 was not covered by tests
}
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 @@
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 @@
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 @@
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
3 changes: 2 additions & 1 deletion gnovm/pkg/gnolang/type_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ func TestCheckAssignableTo(t *testing.T) {

tests := []struct {
name string
n Node
thehowl marked this conversation as resolved.
Show resolved Hide resolved
xt Type
dt Type
autoNative bool
Expand Down Expand Up @@ -51,7 +52,7 @@ func TestCheckAssignableTo(t *testing.T) {
}
}()
}
checkAssignableTo(tt.xt, tt.dt, tt.autoNative)
checkAssignableTo(tt.n, 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 @@
// 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 @@
for i, pf := range ft.Params {
arg := &argTVs[i]
if arg.T.Kind() == TypeKind {
specifyType(store, lookup, pf.Type, arg.T, arg.GetType())
specifyType(n, store, lookup, pf.Type, arg.T, arg.GetType())
} else {
specifyType(store, lookup, pf.Type, arg.T, nil)
specifyType(n, store, lookup, pf.Type, arg.T, nil)
}
}
// apply specifics to generic params and results.
Expand Down Expand Up @@ -2427,7 +2427,7 @@
// 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(n Node, store Store, lookup map[Name]Type, tmpl Type, spec Type, specTypeval Type) {
thehowl marked this conversation as resolved.
Show resolved Hide resolved
if isGeneric(spec) {
panic("spec must not be generic")
}
Expand All @@ -2441,11 +2441,11 @@
case *PointerType:
switch pt := baseOf(spec).(type) {
case *PointerType:
specifyType(store, lookup, ct.Elt, pt.Elt, nil)
specifyType(n, store, lookup, ct.Elt, pt.Elt, nil)

Check warning on line 2444 in gnovm/pkg/gnolang/types.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/types.go#L2444

Added line #L2444 was not covered by tests
case *NativeType:
// NOTE: see note about type-checking.
et := pt.Elem()
specifyType(store, lookup, ct.Elt, et, nil)
specifyType(n, store, lookup, ct.Elt, et, nil)

Check warning on line 2448 in gnovm/pkg/gnolang/types.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/types.go#L2448

Added line #L2448 was not covered by tests
default:
panic(fmt.Sprintf(
"expected pointer kind but got %s",
Expand All @@ -2454,11 +2454,11 @@
case *ArrayType:
switch at := baseOf(spec).(type) {
case *ArrayType:
specifyType(store, lookup, ct.Elt, at.Elt, nil)
specifyType(n, store, lookup, ct.Elt, at.Elt, nil)

Check warning on line 2457 in gnovm/pkg/gnolang/types.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/types.go#L2457

Added line #L2457 was not covered by tests
case *NativeType:
// NOTE: see note about type-checking.
et := at.Elem()
specifyType(store, lookup, ct.Elt, et, nil)
specifyType(n, store, lookup, ct.Elt, et, nil)

Check warning on line 2461 in gnovm/pkg/gnolang/types.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/types.go#L2461

Added line #L2461 was not covered by tests
default:
panic(fmt.Sprintf(
"expected array kind but got %s",
Expand All @@ -2469,7 +2469,7 @@
case PrimitiveType:
if isGeneric(ct.Elt) {
if st.Kind() == StringKind {
specifyType(store, lookup, ct.Elt, Uint8Type, nil)
specifyType(n, store, lookup, ct.Elt, Uint8Type, nil)

Check warning on line 2472 in gnovm/pkg/gnolang/types.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/types.go#L2472

Added line #L2472 was not covered by tests
} else {
panic(fmt.Sprintf(
"expected slice kind but got %s",
Expand All @@ -2485,11 +2485,11 @@
spec.Kind()))
}
case *SliceType:
specifyType(store, lookup, ct.Elt, st.Elt, nil)
specifyType(n, store, 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(n, store, lookup, ct.Elt, et, nil)

Check warning on line 2492 in gnovm/pkg/gnolang/types.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/types.go#L2492

Added line #L2492 was not covered by tests
default:
panic(fmt.Sprintf(
"expected slice kind but got %s",
Expand All @@ -2498,14 +2498,14 @@
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(n, store, lookup, ct.Key, mt.Key, nil)
specifyType(n, store, 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(n, store, lookup, ct.Key, kt, nil)
specifyType(n, store, lookup, ct.Value, vt, nil)

Check warning on line 2508 in gnovm/pkg/gnolang/types.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/types.go#L2507-L2508

Added lines #L2507 - L2508 were not covered by tests
default:
panic(fmt.Sprintf(
"expected map kind but got %s",
Expand Down Expand Up @@ -2544,7 +2544,7 @@
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 @@
} 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 @@
switch cbt := baseOf(spec).(type) {
case *NativeType:
gnoType := store.Go2GnoType(cbt.Type)
specifyType(store, lookup, ct.Type, gnoType, nil)
specifyType(n, store, lookup, ct.Type, gnoType, nil)

Check warning on line 2579 in gnovm/pkg/gnolang/types.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/types.go#L2579

Added line #L2579 was not covered by tests
default:
specifyType(store, lookup, ct.Type, cbt, nil)
specifyType(n, store, 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