Skip to content

Commit

Permalink
feat: gno type check (#1426)
Browse files Browse the repository at this point in the history
**Pinned Update:**

The original #1426 is now divided
into 4 parts, with the dependency relationship being:
#1426 <
#1775,
#1426 <-
https://github.com/gnolang/gno/pull/1890<-
#1891.

Among these, the main part, #1426,
has been supplemented and optimized for the missing parts in the type
checks of the original implementation, specifically as follows:

- A new layer for type check is added(type_check.go). during the
preprocess stage, the compatibility of operators and operands in
expressions is checked, such as 1 - "a". This part used to be
implemented as a runtime error, but now it is checked in type_check.go;
 
- Modifications have been made to checkOrConvertType to add conversion
checks for constants, such as int(1) + int64(1), which previously would
not trigger a compile-time error;

- Refined and improved several aspects of the handling logic for
BinaryExpr during the preprocessing stage.
 
- The existing checkType has been renamed to assertAssignableTo.

==========================update complete=======================

### Problem Definition

Please proceed to #1424.

======update:
fix #1462 , tests located in `gnovm/tests/files/type2`.
this issue is fixed since they share the same contexts of type check and
conversion.

briefly for #1462, type of shift expression (or any composed expression
involved shift expression) will be determined in the context they are
used if they are untyped, also can be mutated by explicitly conversion
with a `type call`.


==========================================================================================


### Overview of Solution
#### checkOperandWithOp function:
**Purpose**: Newly introduced to evaluate operand compatibility before
deep type analysis.
**Functionality**: Employs predefined rules to quickly identify
incompatible patterns (e.g., "a" << 1 is flagged as incompatible).
**Advantage**: Prevents unnecessary processing by checkOrConvertType for
clear mismatches.

#### checkOrConvertType function:
**Role**: Engages after checkOperandWithOp's clearance. It's the hub for
core type checking and conversion.
**Key Improvement**: Enhanced handling of const conversions by limiting
it within a certain range.
**Example**: In cases like int(1) + int(8), the issue of unregulated
const conversion is addressed.
**Constraints**: Mandatory const conversion is now limited to specific
scenarios (e.g., explicit conversion, operand in array/slice index, RHS
of a shift expression).

### Specific Problems Solved

1. **assignable and sameType check:**
This code should output "something else". the root cause for this is
Error(0) is assignable to errCmp since it satisfies the interface of
error, and result in inequality since the have different concrete type
in runtime.
Thanks @jaekwon for pointing out my mistake and give an improved version
of this.

```go
package main

import (
    "errors"
    "strconv"
)

type Error int64

func (e Error) Error() string {
    return "error: " + strconv.Itoa(int(e))
}

var errCmp = errors.New("XXXX")

func main() {
    if Error(0) == errCmp {
        println("what the firetruck?")
    } else {
        println("something else")
    }
}
```


2. **Early Incompatibility Detection:**
Conducted during preprocessing, not runtime.
**Example**:
```go
package main
func main() {
    println(1 / "a") // Detects incompatibility early.
}
```
```go
func main() {
    println(int(1) == int8(1))  // this is checked before checkOrConvertType if LHS and RHS are both typed.
}
```

~~3. **Implicit Conversion:**~~(this is split out)
~~Focus: Ensuring accurate conversions, particularly unnamed to named
types.~~
~~Example:~~
~~```go~~
~~package main~~
~~type word uint~~
~~type nat []word~~
~~func (n nat) add() bool {~~
   ~~ return true~~
~~}
~~func Gen() nat {~~
    ~~n := []word{0}~~
    ~~return n~~
~~}~~
~~func main() {~~
    ~~r := Gen()~~
    ~~switch r.(type) {~~
   ~~ case nat:~~
        ~~println("nat")~~
        ~~println(r.add())~~
    ~~default:~~
        ~~println("should not happen")~~
   ~~ }~~
~~}~~
~~```~~
~~4. **Type of Shift Expressions:**~~
~~**Context**: Determines the type based on usage context and explicit
conversions.~~
~~**Implementation**: Additional checks in assignStmt, callExpr for
potential untyped shift expressions (or else expressions with untyped
shift expression embedded)~~~~appear, e.g. uint64(1 << x). This will
trigger a potentially recursive check&convert until the shift expr got
its final type.~~

### Conclusion:
This PR enhances the type check workflow and addresses previously
overlooked aspects, resolving a variety of type-related issues.

---------

Co-authored-by: Morgan <[email protected]>
Co-authored-by: jaekwon <[email protected]>
  • Loading branch information
3 people authored Jun 19, 2024
1 parent ea969b3 commit 0ba53ac
Show file tree
Hide file tree
Showing 334 changed files with 5,606 additions and 529 deletions.
5 changes: 3 additions & 2 deletions examples/gno.land/p/demo/memeland/memeland_test.gno
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"
"time"

"gno.land/p/demo/seqid"
"gno.land/p/demo/testutils"
"gno.land/p/demo/ufmt"
)
Expand Down Expand Up @@ -97,7 +98,7 @@ func TestGetPostsInRangeByTimestamp(t *testing.T) {

// Count the number of posts returned in the JSON string as a rudimentary check for correct pagination/filtering
postCount := strings.Count(jsonStr, `"id":"`)
if postCount != m.MemeCounter {
if seqid.ID(postCount) != m.MemeCounter {
t.Errorf("Expected %d posts in the JSON string, but found %d", m.MemeCounter, postCount)
}

Expand Down Expand Up @@ -157,7 +158,7 @@ func TestGetPostsInRangeByUpvote(t *testing.T) {

// Count the number of posts returned in the JSON string as a rudimentary check for correct pagination/filtering
postCount := strings.Count(jsonStr, `"id":"`)
if postCount != m.MemeCounter {
if seqid.ID(postCount) != m.MemeCounter {
t.Errorf("Expected %d posts in the JSON string, but found %d", m.MemeCounter, postCount)
}

Expand Down
10 changes: 10 additions & 0 deletions gno.land/pkg/sdk/vm/vm.proto
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ message m_call {
repeated string args = 5;
}

message m_run {
string caller = 1;
string send = 2;
std.MemPackage package = 3;
}

message m_addpkg {
string creator = 1;
std.MemPackage package = 2;
Expand All @@ -28,4 +34,8 @@ message InvalidStmtError {
}

message InvalidExprError {
}

message TypeCheckError {
repeated string errors = 1 [json_name = "Errors"];
}
3 changes: 3 additions & 0 deletions gnovm/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ _test.gnolang.native:; go test tests/*.go -test.short -run "TestFilesNativ
_test.gnolang.stdlibs:; go test tests/*.go -test.short -run 'TestFiles$$/' $(GOTEST_FLAGS)
_test.gnolang.native.sync:; go test tests/*.go -test.short -run "TestFilesNative/" --update-golden-tests $(GOTEST_FLAGS)
_test.gnolang.stdlibs.sync:; go test tests/*.go -test.short -run 'TestFiles$$/' --update-golden-tests $(GOTEST_FLAGS)
# NOTE: challenges are current GnoVM bugs which are supposed to fail.
# If any of these tests pass, it should be moved to a normal test.
_test.gnolang.challenges:; go test tests/*.go -test.short -run 'TestChallenges$$/' $(GOTEST_FLAGS)

########################################
# Code gen
Expand Down
9 changes: 5 additions & 4 deletions gnovm/pkg/gnolang/debugger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ func evalTest(debugAddr, in, file string) (out, err string) {
stdout := writeNopCloser{bout}
stderr := writeNopCloser{berr}
debug := in != "" || debugAddr != ""
mode := tests.ImportModeNativePreferred
if strings.HasSuffix(file, "_stdlibs.gno") {
mode = tests.ImportModeStdlibsPreferred
mode := tests.ImportModeStdlibsPreferred
if strings.HasSuffix(file, "_native.gno") {
mode = tests.ImportModeNativePreferred
}

defer func() {
Expand Down Expand Up @@ -196,7 +196,8 @@ func TestRemoteDebug(t *testing.T) {
func TestRemoteError(t *testing.T) {
_, err := evalTest(":xxx", "", debugTarget)
t.Log("err:", err)
if !strings.Contains(err, "tcp/xxx: unknown port") {
if !strings.Contains(err, "tcp/xxx: unknown port") &&
!strings.Contains(err, "tcp/xxx: nodename nor servname provided, or not known") {
t.Error(err)
}
}
2 changes: 2 additions & 0 deletions gnovm/pkg/gnolang/gnolang.proto
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ message FuncValue {
google.protobuf.Any closure = 5 [json_name = "Closure"];
string file_name = 6 [json_name = "FileName"];
string pkg_path = 7 [json_name = "PkgPath"];
string native_pkg = 8 [json_name = "NativePkg"];
string native_name = 9 [json_name = "NativeName"];
}

message MapValue {
Expand Down
2 changes: 1 addition & 1 deletion gnovm/pkg/gnolang/gonative.go
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ func gno2GoType(t Type) reflect.Type {

// If gno2GoTypeMatches(t, rt) is true, a t value can
// be converted to an rt native value using gno2GoValue(v, rv).
// This is called when autoNative is true in checkType().
// This is called when autoNative is true in assertAssignableTo().
// This is used for all native function calls, and also
// for testing whether a native value implements a gno interface.
func gno2GoTypeMatches(t Type, rt reflect.Type) (result bool) {
Expand Down
18 changes: 9 additions & 9 deletions gnovm/pkg/gnolang/op_assign.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (m *Machine) doOpAddAssign() {
rv := m.PopValue() // only one.
lv := m.PopAsPointer(s.Lhs[0])
if debug {
assertSameTypes(lv.TV.T, rv.T)
debugAssertSameTypes(lv.TV.T, rv.T)
}

// XXX HACK (until value persistence impl'd)
Expand All @@ -73,7 +73,7 @@ func (m *Machine) doOpSubAssign() {
rv := m.PopValue() // only one.
lv := m.PopAsPointer(s.Lhs[0])
if debug {
assertSameTypes(lv.TV.T, rv.T)
debugAssertSameTypes(lv.TV.T, rv.T)
}

// XXX HACK (until value persistence impl'd)
Expand All @@ -96,7 +96,7 @@ func (m *Machine) doOpMulAssign() {
rv := m.PopValue() // only one.
lv := m.PopAsPointer(s.Lhs[0])
if debug {
assertSameTypes(lv.TV.T, rv.T)
debugAssertSameTypes(lv.TV.T, rv.T)
}

// XXX HACK (until value persistence impl'd)
Expand All @@ -119,7 +119,7 @@ func (m *Machine) doOpQuoAssign() {
rv := m.PopValue() // only one.
lv := m.PopAsPointer(s.Lhs[0])
if debug {
assertSameTypes(lv.TV.T, rv.T)
debugAssertSameTypes(lv.TV.T, rv.T)
}

// XXX HACK (until value persistence impl'd)
Expand All @@ -142,7 +142,7 @@ func (m *Machine) doOpRemAssign() {
rv := m.PopValue() // only one.
lv := m.PopAsPointer(s.Lhs[0])
if debug {
assertSameTypes(lv.TV.T, rv.T)
debugAssertSameTypes(lv.TV.T, rv.T)
}

// XXX HACK (until value persistence impl'd)
Expand All @@ -165,7 +165,7 @@ func (m *Machine) doOpBandAssign() {
rv := m.PopValue() // only one.
lv := m.PopAsPointer(s.Lhs[0])
if debug {
assertSameTypes(lv.TV.T, rv.T)
debugAssertSameTypes(lv.TV.T, rv.T)
}

// XXX HACK (until value persistence impl'd)
Expand All @@ -188,7 +188,7 @@ func (m *Machine) doOpBandnAssign() {
rv := m.PopValue() // only one.
lv := m.PopAsPointer(s.Lhs[0])
if debug {
assertSameTypes(lv.TV.T, rv.T)
debugAssertSameTypes(lv.TV.T, rv.T)
}

// XXX HACK (until value persistence impl'd)
Expand All @@ -211,7 +211,7 @@ func (m *Machine) doOpBorAssign() {
rv := m.PopValue() // only one.
lv := m.PopAsPointer(s.Lhs[0])
if debug {
assertSameTypes(lv.TV.T, rv.T)
debugAssertSameTypes(lv.TV.T, rv.T)
}

// XXX HACK (until value persistence impl'd)
Expand All @@ -234,7 +234,7 @@ func (m *Machine) doOpXorAssign() {
rv := m.PopValue() // only one.
lv := m.PopAsPointer(s.Lhs[0])
if debug {
assertSameTypes(lv.TV.T, rv.T)
debugAssertSameTypes(lv.TV.T, rv.T)
}

// XXX HACK (until value persistence impl'd)
Expand Down
34 changes: 17 additions & 17 deletions gnovm/pkg/gnolang/op_binary.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (m *Machine) doOpLor() {
rv := m.PopValue()
lv := m.PeekValue(1) // also the result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// set result in lv.
Expand All @@ -60,7 +60,7 @@ func (m *Machine) doOpLand() {
rv := m.PopValue()
lv := m.PeekValue(1) // also the result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// set result in lv.
Expand All @@ -77,7 +77,7 @@ func (m *Machine) doOpEql() {
rv := m.PopValue()
lv := m.PeekValue(1) // also the result
if debug {
assertEqualityTypes(lv.T, rv.T)
debugAssertEqualityTypes(lv.T, rv.T)
}

// set result in lv.
Expand All @@ -94,7 +94,7 @@ func (m *Machine) doOpNeq() {
rv := m.PopValue()
lv := m.PeekValue(1) // also the result
if debug {
assertEqualityTypes(lv.T, rv.T)
debugAssertEqualityTypes(lv.T, rv.T)
}

// set result in lv.
Expand All @@ -111,7 +111,7 @@ func (m *Machine) doOpLss() {
rv := m.PopValue()
lv := m.PeekValue(1) // also the result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// set the result in lv.
Expand All @@ -128,7 +128,7 @@ func (m *Machine) doOpLeq() {
rv := m.PopValue()
lv := m.PeekValue(1) // also the result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// set the result in lv.
Expand All @@ -145,7 +145,7 @@ func (m *Machine) doOpGtr() {
rv := m.PopValue()
lv := m.PeekValue(1) // also the result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// set the result in lv.
Expand All @@ -162,7 +162,7 @@ func (m *Machine) doOpGeq() {
rv := m.PopValue()
lv := m.PeekValue(1) // also the result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// set the result in lv.
Expand All @@ -179,7 +179,7 @@ func (m *Machine) doOpAdd() {
rv := m.PopValue()
lv := m.PeekValue(1) // also result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// add rv to lv.
Expand All @@ -193,7 +193,7 @@ func (m *Machine) doOpSub() {
rv := m.PopValue()
lv := m.PeekValue(1) // also result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// sub rv from lv.
Expand All @@ -207,7 +207,7 @@ func (m *Machine) doOpBor() {
rv := m.PopValue()
lv := m.PeekValue(1) // also result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// lv | rv
Expand All @@ -221,7 +221,7 @@ func (m *Machine) doOpXor() {
rv := m.PopValue()
lv := m.PeekValue(1) // also result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// lv ^ rv
Expand All @@ -235,7 +235,7 @@ func (m *Machine) doOpMul() {
rv := m.PopValue()
lv := m.PeekValue(1) // also result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// lv * rv
Expand All @@ -249,7 +249,7 @@ func (m *Machine) doOpQuo() {
rv := m.PopValue()
lv := m.PeekValue(1) // also result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// lv / rv
Expand All @@ -263,7 +263,7 @@ func (m *Machine) doOpRem() {
rv := m.PopValue()
lv := m.PeekValue(1) // also result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// lv % rv
Expand Down Expand Up @@ -309,7 +309,7 @@ func (m *Machine) doOpBand() {
rv := m.PopValue()
lv := m.PeekValue(1) // also result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// lv & rv
Expand All @@ -323,7 +323,7 @@ func (m *Machine) doOpBandn() {
rv := m.PopValue()
lv := m.PeekValue(1) // also result
if debug {
assertSameTypes(lv.T, rv.T)
debugAssertSameTypes(lv.T, rv.T)
}

// lv &^ rv
Expand Down
2 changes: 1 addition & 1 deletion gnovm/pkg/gnolang/op_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,7 @@ func (m *Machine) doOpSwitchClauseCase() {

// eval whether cv == tv.
if debug {
assertEqualityTypes(cv.T, tv.T)
debugAssertEqualityTypes(cv.T, tv.T)
}
match := isEql(m.Store, cv, tv)
if match {
Expand Down
Loading

0 comments on commit 0ba53ac

Please sign in to comment.