fix(gnovm): prevent cyclic references in struct declarations

[ltzmaxwell]

Address #2008.

In this pull request, we're implementing a special handling for struct declarations. Due to their unique nature, we must meticulously search through their fields to identify potential cyclic reference.

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
• [ ] Added new benchmarks to generated graphs, if any. More info here.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 97.05882% with 2 lines in your changes missing coverage. Please review.

Project coverage is 60.21%. Comparing base (1f58ee4) to head (284805f). Report is 14 commits behind head on master.

Files	Patch %	Lines
gnovm/pkg/gnolang/preprocess.go	97.05%	1 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2081      +/-   ##
==========================================
+ Coverage   60.11%   60.21%   +0.09%     
==========================================
  Files         560      561       +1     
  Lines       74911    75060     +149     
==========================================
+ Hits        45036    45198     +162     
+ Misses      26500    26485      -15     
- Partials     3375     3377       +2     

Flag	Coverage Δ
contribs/gnodev	61.40% <ø> (ø)
contribs/gnofaucet	14.46% <ø> (ø)
gno.land	64.75% <ø> (+0.56%)	:arrow_up:
gnovm	64.29% <97.05%> (+0.15%)	:arrow_up:
misc/genstd	80.54% <ø> (ø)
misc/logos	19.88% <ø> (ø)
tm2	62.07% <ø> (+0.10%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[petar-dambovaliev]

This might be solvable by adding some logic in transcribe, without a dependency graph.

This might be a better place than transcribe.

var runDeclarationFor func(fn *FileNode, decl Decl)

I don't know. Try it out and see what is the most appropriate place for this.

[ltzmaxwell]

This might be solvable by adding some logic in transcribe, without a dependency graph.

This might be a better place than transcribe.

var runDeclarationFor func(fn *FileNode, decl Decl)

I don't know. Try it out and see what is the most appropriate place for this.

This is great and proves to be the right way to go. It's already updated, please take a look @petar-dambovaliev .

[ltzmaxwell]

The following recursive case is not detected:

package main

type S struct {
    A []F
}

type F func()*S

func main() {
    var a, b S
    println(a == b)
}

// Should fail due to circular dependency

this actually does not lead to cycle dependency. even this:

package main

import "fmt"

type S struct {
	A []S
}

func main() {
	var a, b S

	fmt.Println(a)
	fmt.Println(b)

}

but this will:

package main

import "fmt"

type S struct {
	A [2]S
}

func main() {
	var a, b S

	fmt.Println(a)
	fmt.Println(b)

}

The above cases are all covered, please proceed to the test files. But still thinking if it's the best way to solve this issue.(A dependency graph is introduced during PredefineFileSet stage，seems a bit heavy for this issue. )

[ltzmaxwell]

this PR is updated:

1. Conduct the check in the preprocessing stage, more specifically, when predefineNow2. The advantage of this approach is that it allows checks to be performed at the very beginning, avoiding the issue of fragmented checks throughout later stages;
2. Compared to the previous implementation, it avoids introducing a new dependency graph to solve the problem, making the solution relatively streamlined.

@mvertes @petar-dambovaliev @thehowl @ajnavarro would you please conduct a second review, Thank you! 🙏

[ltzmaxwell]

@ltzmaxwell the code looks goods and testing cases are well defined.

Have we considered this cyclic dependency case? The Go compiler caught it.

package main

func main() {

}

var a = f1()
var b = f2()

func f1() int {
	return b + 1
}

func f2() int {
	return a + 1
}

// https://go.dev/play/p/FqTP7KNKQ0E?v=goprev

$ go tool compile cyclefunc.go cyclefunc.go:7:5: initialization cycle for a cyclefunc.go:7:5: a refers to cyclefunc.go:10:6: f1 refers to cyclefunc.go:8:5: b refers to cyclefunc.go:14:6: f2 refers to cyclefunc.go:7:5: a

@piux2 Thank you for your review! This particular case isn't included in the current PR, as it focuses on cycles in type declarations, whereas the issue above pertains to value declarations. Nonetheless, it's an excellent catch. I propose we track it with a separate issue and address it in a subsequent PR, what do you think?