gnolang / gno

Gno: An interpreted, stack-based Go virtual machine to build succinct and composable apps + Gno.land: a blockchain for timeless code and fair open-source.
https://gno.land/
Other
880 stars 364 forks source link

Add recommendation to Effective Gno: limit `init` to only being declarable once #1482

Open waymobetta opened 9 months ago

waymobetta commented 9 months ago

Description

Similarly to Go, a .gno file can support multiple init functions within the same file. This is odd to see though, apparently, is completely valid; deploys, runs and is able to be queried.

Playground example

Result: data: ("bar" std.Address), meaning it operates like the LIFO (last-in; first-out) principle.

Perhaps, to avoid confusion, and to further differentiate it from Go's init, we make the Gno init behave more like the main function in that it can only be declared once.

TODO:

waymobetta commented 8 months ago

Furthering the conversation on this, since we are not using this in the same way as Go- rather we are using it as a constructor function to execute only a single time during its lifecycle (at deployment)- having multiple constructors could actually be problematic since those coming from other smart contract languages (eg, Solidity) are accustomed to only declaring a single constructor function within their application, despite having multiple files; in other words, there is no constructor "overloading" allowed.

If we choose to continue allowing constructor "overloading," at the very least we should make it apparent in the documentation that whatever data is set within these constructors may get overwritten (especially if the same variable is mentioned) which could lead to unintended consequences or, at least, create a confusing order of operations when determining which init executes before the other if there are multiple found present in an app.

thehowl commented 8 months ago

Just a quick note: there will almost certainly always be ways to create "multiple constructor functions", even if we restrict init to being declarable only once.

The reason is that there is a ""secret"" alternative way to express the init function:

package hello

func main() {
  println(x)
}

var x int

var _ = func() int {
  x = 11
  return 0
}()

playground

This will output 11 -- and is intended behaviour.

Variables can be initialised to any expression without restriction per the Go spec (excluding cyclical references, whereby then initialization order could not be determined).

So, while I get your point in theory, unless we also change variable initialization radically for Gno (such as, disabling it entirely, which I don't think is a good idea) there will always be ways for people to have "hidden initialisation side-effects" in other files.

waymobetta commented 8 months ago

That's true, but your example isn't a widely used concept and one that is expressed within Go standard documentation (+ Effective Go); in other words, though it works, it feels more like a hack.

What I am suggesting is taking what Go programmers ordinarily understand to be normal behavior (multiple init scattered throughout their app for db initializations, etc.) and restricting it to a single declaration so they don't do something which could create unintended consequences because the behavior of our init is different than that of Go's.

thehowl commented 8 months ago

I'm not completely opposed. It does simplify some things in many areas, and regardless of what road we undertake we should encourage having at most one init function, be that through the linter and/or Effective Gno.

That being said, we still need to consider that:

For this reason, I think it makes more sense to have init declarable once as a recommendation rather than a language change.

waymobetta commented 8 months ago

Yep, I hear what you are saying in terms of deviating from Go spec and I like that recommendation as a good middleground without introducing further complications. It's good we've had a discussion on this to refer back to and support what is found in Effective Gno/best practices/recommendations.