feat(gnovm): sync code AssignStmt - ValueDecl

[hthieu1110]

This PR aims at fixing this issue 1958

Contributors' checklist...

• 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

[codecov]

Codecov Report

Attention: Patch coverage is 97.20280% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
gnovm/pkg/gnolang/preprocess.go	97.20%	3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[MikaelVallenet]

Looks good to me 🔥

[ltzmaxwell]

When handling assignStmt and valueDecl, there are two main tasks:

• Ensuring the compatibility of the statement(in type_check.go)

  gno/gnovm/pkg/gnolang/type_check.go

  Line 838 in 7ef606c

  func (x *AssignStmt) AssertCompatible(store Store, last BlockNode) {

• Performing declarations for nameExprs(in preprocess.go, defineOrDecl)

To unify the specifications for assignStmt and valueDecl, these tasks should be handled consistently for both.

For example:

package main

func main() {
    var a int = "hello" // valueDecl

    var a int
    a = "hello" // assignStmt
}

In this code, both valueDecl and assignStmt should follow the same logic during type checking to ensure compatibility(in type_check.go).

To summarize, we should apply both AssertCompatible(step1) and defineOrDecl (step2) operations to handle assignStmt and valueDecl consistently.

what do you think?

[ltzmaxwell]

Related: #2695

[hthieu1110]

When handling assignStmt and valueDecl, there are two main tasks:

• Ensuring the compatibility of the statement(in type_check.go)

  gno/gnovm/pkg/gnolang/type_check.go

  Line 838 in 7ef606c

  func (x *AssignStmt) AssertCompatible(store Store, last BlockNode) {

• Performing declarations for nameExprs(in preprocess.go, defineOrDecl)

To unify the specifications for assignStmt and valueDecl, these tasks should be handled consistently for both.

For example:

package main

func main() {
    var a int = "hello" // valueDecl

    var a int
    a = "hello" // assignStmt
}

In this code, both valueDecl and assignStmt should follow the same logic during type checking to ensure compatibility(in type_check.go).

To summarize, we should apply both AssertCompatible(step1) and defineOrDecl (step2) operations to handle assignStmt and valueDecl consistently.

what do you think?

Thanks for your review.

I've checked the code, there is only one re-usable code in AssignStmt assertCompatible is:

if numNames != len(cft.Results) {
panic(fmt.Sprintf(
	"assignment mismatch: "+
		"%d variables but %s returns %d values",
	numNames, cx.Func.String(), len(cft.Results)))
}

Which is already handled in defineOrDecl.

Other processings are mostly for Assign.

I've tried to refactor to be able to share the code even more (refactor/reuse assertCompatible) but don't find any "happy/good" solution. Do you have any idea on that ?

[jefft0]

Removed the review/triage-pending label because this PR was reviewed by core devs mvertes and ltzmaxwell.

[jefft0]

Hello @hthieu1110 . Many of the CI checks failed because of a misconfiguration. This has now been fixed. Can you please push an empty commit? This will make the CI checks run again and hopefully fix them.

[Kouteki]

Can you please push an empty commit? This will make the CI checks run again and hopefully fix them.

We're having issues with Codecov, ignore failed Codecov tests for now.

[hthieu1110]

When handling assignStmt and valueDecl, there are two main tasks:

• Ensuring the compatibility of the statement(in type_check.go)

  gno/gnovm/pkg/gnolang/type_check.go

  Line 838 in 7ef606c

  func (x *AssignStmt) AssertCompatible(store Store, last BlockNode) {

• Performing declarations for nameExprs(in preprocess.go, defineOrDecl)

To unify the specifications for assignStmt and valueDecl, these tasks should be handled consistently for both.
For example:

package main

func main() {
    var a int = "hello" // valueDecl

    var a int
    a = "hello" // assignStmt
}

In this code, both valueDecl and assignStmt should follow the same logic during type checking to ensure compatibility(in type_check.go).
To summarize, we should apply both AssertCompatible(step1) and defineOrDecl (step2) operations to handle assignStmt and valueDecl consistently.
what do you think?

Thanks for your review.

I've checked the code, there is only one re-usable code in AssignStmt assertCompatible is:

if numNames != len(cft.Results) {
panic(fmt.Sprintf(
	"assignment mismatch: "+
		"%d variables but %s returns %d values",
	numNames, cx.Func.String(), len(cft.Results)))
}

Which is already handled in defineOrDecl.

Other processings are mostly for Assign.

I've tried to refactor to be able to share the code even more (refactor/reuse assertCompatible) but don't find any "happy/good" solution. Do you have any idea on that ?

Could you take a look on this pls :) thanks

[ltzmaxwell]

sorry keep you waiting, gonna finalize my review tomorrow. 🙏

[ltzmaxwell]

mostly nits, but otherwise LGTM. thank you for the work.

[mvertes]

look good to me. Thanks for addressing my remarks.

[ltzmaxwell]

merge this, cc:/ @thehowl if you want review this in future.