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

[META] improved native bindings #814

Open
4 of 8 tasks
thehowl opened this issue May 9, 2023 · 13 comments
Open
4 of 8 tasks

[META] improved native bindings #814

thehowl opened this issue May 9, 2023 · 13 comments

Comments

@thehowl
Copy link
Member

thehowl commented May 9, 2023

This issue has been hijacked from its original proposal to point to a "roadmap" of tasks to be done in the context of the below proposal, following the merge of the initial implementation in #859. The original, full proposal can be found below.

  • Decide on a better approach to recovering previous StaticBlocks when precompiling fails: refactor(gnolang): handle duplicate method decls using TryDefineMethod #1459
  • Support precompilation of native functions by linking directly to the Go source in stdlibs/, removing the necessity of stdshim. feat(transpiler): transpile gno standard libraries #1695
  • Supporting the test-only tests/stdlibs/ directory in gno doc; as well as documenting native functions explicitly by adding the // injected comment after their declaration.
    Note: probably will be removed, see feat: std re-organization #2425
  • Bring over the strconv package to a Gno implementation; this way in tests/imports.go the special packages and native values are restricted only to ImportModeNativePreferred, while everything else uses documentable native bindings also for tests. Additionally, the "package injector" system can officially be removed after this is done. feat(stdlibs): add package strconv #1464
  • Document more standard library functions, especially in std! (feat: std re-organization #2425)
  • Understand //gno:link as a "compiler directive" for types like std.Address, which are currently manually added as mappings in misc/genstd/mapping.go (no longer necessary)
  • Create shims for native testing functions which are currently unsupported
  • Improve performance/codegen of natively bound functions

Eventually, connected to this endeavour but a bit separate, we should shoot for removing all usages of NativeType/NativeValue: #1361


Proposal

This is an issue to propose and discuss a better integration and implementation of native Go bindings in the Gno standard libraries, trying to remove places for pitfalls and for erroneous behaviour while making it easier to contribute and expand functionality in the standard libraries.

Background

See also: #808, #668, #522

The standard library is seemingly designed to allow for running Gno code both in a context where stdlibs are loaded as .gno files from the gnovm/stdlibs directory, as native bindings (mostly useful for the filetests in gnovm/tests), and also where code is precompiled. This is a bit disorganised in different places, and has generated the issues mentioned above.

  • stdlibWhitelist in gnovm/pkg/gnolang/precompile.go has to be kept in sync with the libraries from the Go stdlib which we have implemented. Although furthermore we also have to specify custom rules on how to handle the "std" package, and crucially would currently require more manual intervention if we wanted to precompile ie packages importing crypto/chacha20 (which is a custom Gno library).
  • Many functions implemented in stdlibs/stdlibs.go are defined using pn.DefineNative when this is not really required because they don't intrinsically need to know about the context, but they do something which would be hard/slow to do in pure gno, with such an example being the sha256.Sum256 function.
  • The test environment exposes more functions in a manner that is not really transparent - these are defined in gnovm/tests/imports.go, including packages like os, fmt, base64, etc. While it largely makes sense for these not to be exposed in standard gnovm contexts, it should be (1) be more visible to tooling (like gno doc) that these functions exist and that (2) they are available when writing tests.

Proposal

  • I propose to remove native bindings the way they are done right now, and instead use a similar mechanism's to Go's with assembly code: if a function only has a declaration without a body, then it is expected to be implemented in Go.
    • Because of Gno's interpreted nature, this is only expected and supported in the standard library, where we can create tooling to do static code analysis and pregenerate all bindings to native Go functions.
    • Additionally, a new set of standard libraries is added to tests/stdlibs, which exposes the functions that are overridden or added in test contexts (gno test, gno run...).
  • Declared Gno functions may be implemented in Go code by functions which have the same function signature (ie. arguments of the same type, but allowing for arguments with different name).
    • Unexported Gno functions may also have a corresponding native function, which should however have the name prefixed with X_ (ie. func println -> func X_println). This is to allow the function to be exported at a go level, and callable from another package.
  • Additionally, another accepted signature is prefixing the function's arguments with the argument *gno.Machine (of pkg gnovm/pkg/gnolang). This allows to access gnovm internals in the same way that it is currently done in gnovm/stdlibs/stdlibs.go.
  • If a non-stdlib type is used (ie. a named type or a complex type that references a named type), then the named types used should have a matching Go binding, through use of the //gno:link <gnoType> [<goPkg>.]<goType> compiler directive (see below for an example), in a similar fashion to how Go has //go:linkname.
  • This has the advantage of maintaining a single source of truth for the standard libraries to find all the exported symbols available, and is transparent to documentation and AST-based tools like gno doc, and any possible future autocompletion tools where there won't be a need to add ad-hoc exceptions for native functions.
  • The precompiler performs three kind of translations from a native function call to the precompiled Go version. See below for an example of all three.
    1. If the corresponding native function is of the kind func (*gno.Machine, ...), as described above, then the package is added as an import and the function is called with the first argument (*gno.Machine) set to nil.
    2. If the corresponding native function is a single-instruction return statement, the call to the native function is substituted with the value of that return statement.
    3. Otherwise, like in i., the package is added as an import and the function is called with the matching arguments.

Example std package

// gnovm/stdlibs/std/std.gno
package std

// Examples taken from real-world std functions

//gno:link Address "github.com/gnolang/gno/tm2/pkg/crypto".Bech32Address

type Address string

func EncodeBech32(prefix string, bytes [20]byte) (addr Address)

// Documentation which would show up in `gno doc`...
func GetOrigCaller() Address

func Hash(bz []byte) [20]byte
// gnovm/stdlibs/std/std.go
package std

import (
	gno "github.com/gnolang/gno/tm2/pkg/gnolang"
	"github.com/gnolang/gno/tm2/pkg/crypto"
	"github.com/gnolang/gno/tm2/pkg/bech32"
)

func EncodeBech32(prefix string, bytes [20]byte) crypto.Bech32Address {
	/* Note this check is done in stdlibs/stdlibs.go but would be useless here;
	before calling native functions like this one, where the type is [20]bytes,
	the vm/native calling system would do this check automatically.

	if len(bytes) != crypto.AddressSize {
		panic("should not happen")
	}*/
	b32, err := bech32.ConvertAndEncode(prefix, bytes)
	if err != nil {
		panic(err) // should not happen
	}
	return crypto.Bech32Address(b32)
}

func GetOrigCaller(m *gno.Machine) crypto.Bech32Address {
	if m == nil {
		panic("cannot call std.GetOrigCaller from precompiled code")
	}
	return m.Context.(ExecContext).OrigCaller
}

func Hash(bz []byte) [20]byte {
	return gno.HashBytes(bz)
}

Example user of std package, and generated precompiled code

// examples/gno.land/r/r1/r1.gno
package r1

import "std"

func SampleFunction() {
	std.EncodeBech32("r1", [20]byte{})
	std.Hash([]byte(std.GetOrigCaller())
}
// examples/gno.land/r/r1/r1.gno.gen.go (ie. precompiled)
package r1

import (
	gno "github.com/gnolang/gno/tm2/pkg/gnolang"
	"github.com/gnolang/gno/gnovm/stdlibs/std"
)

func SampleFunction() {
	std.EncodeBech32("r1", [20]byte{})
	// std.Hash is "inlineable" and thus auto-converted to gno.HashBytes.
	// std.GetOrigCaller takes in *gno.Machine, but we're not in the vm now so the machine is nil.
	gno.HashBytes([]byte(std.GetOrigCaller(nil))
}
@tbruyelle
Copy link
Contributor

For someone who has often felt confused by the issues you mention, I really appreciate you took the time to investigate the whole topic and gave us a proposal of improvement!

I like the idea of using the function without body for that. So you can define the body whether in go directly or via the Init function. If you use Init it's because you want to access the context right ?
Maybe it would be nice to allow an alternate function signature that takes the gno.Machine as the first parameter, thus we can remove the injection via Init.
If I retake your example, this would also be valid mapping :

// json.gno
func MarshalJSON(v interface{}) ([]byte, error)

// json.go
import (
  "encoding/json"
  gno "github.com/gnolang/gno/gnovm/pkg/gnolang"
)

func MarshalJSON(m *gno.Machine, v interface{}) ([]byte, error) {
    return json.MarshalJSON(v)
}

But maybe that's a little far fetched and/or wouldn't work properly.

Otherwise, if the native functions are more complex, then the package "$gnoimportpath/gnovm/stdlibs/encoding/json" (for instance) is imported and the native function called.

I still don't really understand why do you mean by that. What the kind of complexity are you referring to?

@thehowl
Copy link
Member Author

thehowl commented May 12, 2023

I like the idea of using the function without body for that. So you can define the body whether in go directly or via the Init function. If you use Init it's because you want to access the context right ? Maybe it would be nice to allow an alternate function signature that takes the gno.Machine as the first parameter, thus we can remove the injection via Init.

That idea also passed through my mind. I guess it could work, though I don't have the expertise to know if adding m *gno.Machine as the first argument covers most cases. From what I can see in stdlibs/stdlibs.go, pn is not used inside of the native functions themselves and store should also be available through m.Store where necessary, so I agree with you that it could be a viable solution, but I'd like to have some extra feedback here.

One concern is that precompilation (I explained better below how that works) would probably have more trouble if they're implemented this way. With the Init function, we can instead provide a Gno version and a precompiled version (like we're doing now for instance for std - where references to the std package are replaced with stdshim when precompiled)

Otherwise, if the native functions are more complex, then the package "$gnoimportpath/gnovm/stdlibs/encoding/json" (for instance) is imported and the native function called.

I still don't really understand why do you mean by that. What the kind of complexity are you referring to?

If the matching native function simply refers, for instance, to another Go standard library function, the easiest thing to do when precompiling Gno code is to replace the call to the function in the Gno stdlib to that in the Go stdlib

// stdlibs/strconv/strconv.gno

func Itoa(i int) string

// stdlibs/strconv/strconv.go

func Itoa(i int) string {
	return strconv.Itoa(i)
}

Then when we use strconv.Itoa in gno code, when we precompile it it is replaced with the go stdlib equivalent.

// example.gno
import "strconv"

func Render() string { return strconv.Itoa(11) }

// example.gno.gen.go
import "strconv"

func Render() string { return strconv.Itoa(11) }

However, say we change Itoa to also have other side effects, say a fmt.Println

// stdlbs/strconv/strconv.go

func Itoa(i int) string {
    fmt.Println("Hello!")
    return strconv.Itoa(i)
}

To maintain consistency between precompiled and gno execution, the precompiled version should become this one:

// example.gno.gen.go
import "github.com/gnolang/gno/gnovm/stdlibs/strconv"

func Render() string { return strconv.Itoa(11) }

@ajnavarro
Copy link
Contributor

I think that maybe we don't need any precompile step to do, and we can just execute native code when interpreting the code on the VM if we detect that the function called is from the standard library. We will need to give to these functions a specific gas fee too.

@thehowl
Copy link
Member Author

thehowl commented May 12, 2023

@ajnavarro we don't do precompile steps in normal executions, but precompilation is a feature supported by cmd/gno and I think we should continue to support.

And yes, indeed while we still need to sort out gas fees for native function it currently works like you said. Except that now everything's in a big file containing all the native functions (mostly stdlibs/stdlibs.go)

@ajnavarro ajnavarro added 📦 🤖 gnovm Issues or PRs gnovm related 🌟 improvement performance improvements, refactors ... labels May 15, 2023
@tbruyelle
Copy link
Contributor

One concern is that precompilation (I explained better below how that works) would probably have more trouble if they're implemented this way.

Yes probably, let's do it with function without body + Init first. Then we'll see if we can simplify with a only function without body.

Thx for the explanation.

@thehowl
Copy link
Member Author

thehowl commented May 16, 2023

One thought @tbruyelle -- I just thought about it and your approach may actually be better and the issue I was referring to easily resolvable. We can simply make calls to the functions which use *gno.Machine use nil when precompiled, so ie. this is how I would implement AssertOriginCall

package std

import // ...

func AssertOriginCall(m *gno.Machine) {
  if m == nil {
    panic("std.AssertOriginCall called from precompiled code")
  }
  if len(m.Frames) != 2 {
    m.Panic("invalid non-origin call")
  }
}

Then when we precompile std.AssertOriginCall() in gno code, it gets converted to std.AssertOriginCall(nil).

@tbruyelle
Copy link
Contributor

Indeed nice approach !
I never touched the precompile part, do you think it's easy to convert method calls like that ?

@thehowl
Copy link
Member Author

thehowl commented May 16, 2023

I'm also not exactly knowledgeable about precompile internals, but starting from an AST tree and pregenerated mappings this should be doable without too much magic :)

@moul
Copy link
Member

moul commented May 17, 2023

Related with #239 and #498

Good idea, but let's create a PoC to make informed decisions for next steps.

@thehowl
Copy link
Member Author

thehowl commented May 17, 2023

I've incorporated the latest ideas in the comments into the OP, and also added a lengthier example section showing exactly how I envision both stdlib code and realm code (and precompiled).

Feedback is very welcome. I'll start working on a PoC soon.

@ajnavarro
Copy link
Contributor

ajnavarro commented May 18, 2023

Leaving a different approach here, just to continue the discussion and checking pros and cons:

using the following example:

Example user of std package, and generated precompiled code

// examples/gno.land/r/r1/r1.gno
package r1

import "std"

func SampleFunction() {
	std.EncodeBech32("r1", [20]byte{})
	std.Hash([]byte(std.GetOrigCaller())
}
// examples/gno.land/r/r1/r1.gno.gen.go (ie. precompiled)
package r1

import (
	"github.com/gnolang/gno/gnovm/stdlibs/std"
)

func SampleFunction() {
	std.EncodeBech32("r1", [20]byte{})
	std.Hash([]byte(std.GetOrigCaller())
}
  • std package is implemented in go, so we can use it as a dependency for go precompiled code, and internally on the VM.
  • There will be no difference between the go and gno code, so the only thing that will change is the import.
  • When code is executed inside the VM, we can use reflection to call functions on specific packages, std in this case.
  • No need to keep track of a huge list of methods on the VM, any change to std package will be reflected on VM and generated go code.
  • not crossed dependencies VM <-> std package

WDYT?

@thehowl
Copy link
Member Author

thehowl commented May 18, 2023

@ajnavarro

One question, from your example: how does std.GetOrigCaller() gather access to *gno.Machine? Especially for the std package, many function calls require access to the VM's data. Otherwise, the proposals look very similar

@ajnavarro
Copy link
Contributor

ajnavarro commented May 18, 2023

We could call something like std.With(vm).GetOrigCaller() from the VM to make it available. When the code is executed from Go, we can leave a default mocked VM as the default or use any other implementation. But these might be ideas for the future. std.GetOrigCaller() will call under the hood to std.With(&MockVM{}).GetOrigCaller() (or a nil VM) when called from Go, and that call to std.GetOrigCaller() will be translated to std.With(vm).GetOrigCaller() when called from Gno VM using reflection.

What I'm trying to do here is to avoid having code generation, and make everything as straightforward as possible to allow future IDEs to autocomplete and things like that.

Feel free to discard the idea if you feel it doesn't fit well (you have a bigger context here than me). I'm just hoping that this proposal helps to validate the main one.

@moul moul added this to the 🌟 main.gno.land (wanted) milestone Sep 8, 2023
thehowl added a commit that referenced this issue Dec 18, 2023
This PR provides an initial transition and groundwork to move from
native bindings as "package injections", whereby an injector function is
called after package parsing and loading to provide built-in natives, to
the new design described in #814.

## Status

The PR is currently ready for review.

- [x] Fix all tests and ensure precompiling has no issues
- [x] Find out if the errors that arise when running the tests with
`DEBUG=1` are bugs introduced by this PR or not
- [x] Create temporary exceptions for linked types (like
`std.{Coins,Address,Realm}`), so that all normal native bindings run
exclusively on the new system, awaiting `//gno:link` (see #814).
- [x] Finding a permanent place for ExecContext (see comment on
`gnovm/stdlibs/context.go`); suggestions welcome (tm2/vm?)
- [x] Include code generation in Makefiles and add a CI check to make
sure PRs do generate code when necessary
- [x] And some unit tests for the new functionality / potential edge
cases.
- [x] make sure all injected functions have `// injected` as a comment

Next steps, after merging this PR, include:

- Supporting precompilation of native functions by linking directly to
the Go source in stdlibs/, removing the necessity of `stdshim`.
- Supporting the test-only tests/stdlibs/ directory in `gno doc`; as
well as documenting native functions explicitly by adding the `//
injected` comment after their declaration.
- Making all of the native-only packages defined in tests/imports.go
native bindings; so that the special packages and native values are
restricted only to ImportModeNativePreferred, while everything else uses
documentable native bindings also for tests.

## Implementation Summary

A new API has been added to `gnolang.Store`: `NativeStore`. This is a
simple function which resolves a package and a name of a function to a
native function.[^1] This is used to resolve native bindings first and
foremost in `preprocess.go`, which also adds information about the
native binding in `NativePkg` and `NativeName`, two new fields in
`*gnolang.FuncValue`.

The purpose of `NativePkg` and `NativeName` is to 1. enable assigning
native functions to other function types, including function parameters
or func type variables and 2. being able to easily and effectively
persist the FuncValue during realm de/serialization. This way,
`op_call.go` can recover the value of `nativeBody` if it is nil.

On the other side, examples of the new stdlibs can be seen in the
`gnovm/stdlibs` directory, where everything except a few functions in
package `std` have been ported to the new system. The code generator in
`misc/genstd` parses the AST of both the Go and Gno files, and
ultimately generates `native.go`, which contains the porting code from
the Gno function call to Go and back.

To support the test contexts, which needs to have different function
implementation or entirely new functions, an additional stdlibs
directory was added to the `tests` directory. This is still code
generated with the same generator and follows the same structure. When
stdlibs are loaded for tests, the code in tests/stdlibs/, if it exists,
is loaded with "higher priority" compared to normal `stdlibs` code.[^2]

### Suggested order for reviewing (and some notes)

1. `pkg/gnolang` for the core of the changes, starting from `values.go`,
`preprocess.go`, `op_call.go`, `store.go`, `realm.go` which show how
NativePkg, NativeName and nativeBody interact
* `gnovm/pkg/gnolang/op_eval.go` adds the type of the evaluated
expression, as I think it's useful to understand in what terms something
is being evaluated; specifically I added it to better distinguish
between `DEBUG: EVAL: (*gnolang.CallExpr)
std<VPBlock(3,0)>.TestCurrentRealm()` and `DEBUG: EVAL:
(*gnolang.SelectorExpr) std<VPBlock(3,0)>.TestCurrentRealm`. I think
it's useful, but we can remove it if we prefer to keep it simpler.
* `gnovm/pkg/gnolang/op_expressions.go` adds a new debug print
pretending we're popping and pushing the value stack. This is to make
clear what's happening in the debug logs (we're really swapping the
value at the pointer), and I've made it `v[S]` so it's clear that the
log is not coming from `PushValue/PopValue`.
* I removed a goto-loop in `gnovm/pkg/gnolang/values.go` because I don't
see it as beneficial when it's essentially just doing a for loop; if the
concern was inlining, even with goto the function is too "costly" for
the go compiler to inline.
2. `misc/genstd` contains the code generator and the template it uses to
generate the files, ie. `gnovm/stdlibs/native.go` and
`gnovm/tests/stdlibs/native.go`.
3. `stdlibs` and `tests/stdlibs` changes show the new stdlib
directories, which genstd uses to generate code. The `tests/stdlibs` is
available only in test environments, and there are also some overriding
definitions to those in normal stdlibs.
* `gnovm/stdlibs/encoding/base64/base64.gno` removes a check on
`strconv.IntSize`. This is because `strconv.IntSize` is dependent on the
system and thus is non-deterministic behaviour on the Gnovm. I don't
know if `int` in Gno is == Go's `int` or it is precisely defined to be
64-bits; in any case I don't think it should be dependent on the system
as this wouldn't work in a blockchain scenario.
4. Changes to `tm2` and `gnovm/tests/imports.go` mostly relate to wiring
up the new NativeStore to the gnovm where it's used.
5. Changes to `examples` and `tests/files` (except for float5) are all
updates to golden tests.

<details><summary>Checklists...</summary>
<p>

## Contributors Checklist

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests

## Maintainers Checklist

- [ ] Checked that the author followed the guidelines in
`CONTRIBUTING.md`
- [ ] Checked the conventional-commit (especially PR title and verb,
presence of `BREAKING CHANGE:` in the body)
- [ ] Ensured that this PR is not a significant change or confirmed that
the review/consideration process was appropriate for the change

</p>
</details> 

[^1]: Currently, the only native bindings that are supported with the
new system are top-level functions. I think we can keep it this way in
the spirit of [KISS](https://en.wikipedia.org/wiki/KISS_principle);
maybe think of extending it if we find appropriate usage.
[^2]: See gnolang.RunMemPackageWithOverrides

---------

Co-authored-by: Thomas Bruyelle <thomas.bruyelle@gmail.com>
Co-authored-by: jaekwon <jae@tendermint.com>
@thehowl thehowl changed the title rfc: better native bindings [META] improved native bindings Dec 19, 2023
gfanton pushed a commit to moul/gno that referenced this issue Jan 18, 2024
This PR provides an initial transition and groundwork to move from
native bindings as "package injections", whereby an injector function is
called after package parsing and loading to provide built-in natives, to
the new design described in gnolang#814.

## Status

The PR is currently ready for review.

- [x] Fix all tests and ensure precompiling has no issues
- [x] Find out if the errors that arise when running the tests with
`DEBUG=1` are bugs introduced by this PR or not
- [x] Create temporary exceptions for linked types (like
`std.{Coins,Address,Realm}`), so that all normal native bindings run
exclusively on the new system, awaiting `//gno:link` (see gnolang#814).
- [x] Finding a permanent place for ExecContext (see comment on
`gnovm/stdlibs/context.go`); suggestions welcome (tm2/vm?)
- [x] Include code generation in Makefiles and add a CI check to make
sure PRs do generate code when necessary
- [x] And some unit tests for the new functionality / potential edge
cases.
- [x] make sure all injected functions have `// injected` as a comment

Next steps, after merging this PR, include:

- Supporting precompilation of native functions by linking directly to
the Go source in stdlibs/, removing the necessity of `stdshim`.
- Supporting the test-only tests/stdlibs/ directory in `gno doc`; as
well as documenting native functions explicitly by adding the `//
injected` comment after their declaration.
- Making all of the native-only packages defined in tests/imports.go
native bindings; so that the special packages and native values are
restricted only to ImportModeNativePreferred, while everything else uses
documentable native bindings also for tests.

## Implementation Summary

A new API has been added to `gnolang.Store`: `NativeStore`. This is a
simple function which resolves a package and a name of a function to a
native function.[^1] This is used to resolve native bindings first and
foremost in `preprocess.go`, which also adds information about the
native binding in `NativePkg` and `NativeName`, two new fields in
`*gnolang.FuncValue`.

The purpose of `NativePkg` and `NativeName` is to 1. enable assigning
native functions to other function types, including function parameters
or func type variables and 2. being able to easily and effectively
persist the FuncValue during realm de/serialization. This way,
`op_call.go` can recover the value of `nativeBody` if it is nil.

On the other side, examples of the new stdlibs can be seen in the
`gnovm/stdlibs` directory, where everything except a few functions in
package `std` have been ported to the new system. The code generator in
`misc/genstd` parses the AST of both the Go and Gno files, and
ultimately generates `native.go`, which contains the porting code from
the Gno function call to Go and back.

To support the test contexts, which needs to have different function
implementation or entirely new functions, an additional stdlibs
directory was added to the `tests` directory. This is still code
generated with the same generator and follows the same structure. When
stdlibs are loaded for tests, the code in tests/stdlibs/, if it exists,
is loaded with "higher priority" compared to normal `stdlibs` code.[^2]

### Suggested order for reviewing (and some notes)

1. `pkg/gnolang` for the core of the changes, starting from `values.go`,
`preprocess.go`, `op_call.go`, `store.go`, `realm.go` which show how
NativePkg, NativeName and nativeBody interact
* `gnovm/pkg/gnolang/op_eval.go` adds the type of the evaluated
expression, as I think it's useful to understand in what terms something
is being evaluated; specifically I added it to better distinguish
between `DEBUG: EVAL: (*gnolang.CallExpr)
std<VPBlock(3,0)>.TestCurrentRealm()` and `DEBUG: EVAL:
(*gnolang.SelectorExpr) std<VPBlock(3,0)>.TestCurrentRealm`. I think
it's useful, but we can remove it if we prefer to keep it simpler.
* `gnovm/pkg/gnolang/op_expressions.go` adds a new debug print
pretending we're popping and pushing the value stack. This is to make
clear what's happening in the debug logs (we're really swapping the
value at the pointer), and I've made it `v[S]` so it's clear that the
log is not coming from `PushValue/PopValue`.
* I removed a goto-loop in `gnovm/pkg/gnolang/values.go` because I don't
see it as beneficial when it's essentially just doing a for loop; if the
concern was inlining, even with goto the function is too "costly" for
the go compiler to inline.
2. `misc/genstd` contains the code generator and the template it uses to
generate the files, ie. `gnovm/stdlibs/native.go` and
`gnovm/tests/stdlibs/native.go`.
3. `stdlibs` and `tests/stdlibs` changes show the new stdlib
directories, which genstd uses to generate code. The `tests/stdlibs` is
available only in test environments, and there are also some overriding
definitions to those in normal stdlibs.
* `gnovm/stdlibs/encoding/base64/base64.gno` removes a check on
`strconv.IntSize`. This is because `strconv.IntSize` is dependent on the
system and thus is non-deterministic behaviour on the Gnovm. I don't
know if `int` in Gno is == Go's `int` or it is precisely defined to be
64-bits; in any case I don't think it should be dependent on the system
as this wouldn't work in a blockchain scenario.
4. Changes to `tm2` and `gnovm/tests/imports.go` mostly relate to wiring
up the new NativeStore to the gnovm where it's used.
5. Changes to `examples` and `tests/files` (except for float5) are all
updates to golden tests.

<details><summary>Checklists...</summary>
<p>

## Contributors Checklist

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests

## Maintainers Checklist

- [ ] Checked that the author followed the guidelines in
`CONTRIBUTING.md`
- [ ] Checked the conventional-commit (especially PR title and verb,
presence of `BREAKING CHANGE:` in the body)
- [ ] Ensured that this PR is not a significant change or confirmed that
the review/consideration process was appropriate for the change

</p>
</details> 

[^1]: Currently, the only native bindings that are supported with the
new system are top-level functions. I think we can keep it this way in
the spirit of [KISS](https://en.wikipedia.org/wiki/KISS_principle);
maybe think of extending it if we find appropriate usage.
[^2]: See gnolang.RunMemPackageWithOverrides

---------

Co-authored-by: Thomas Bruyelle <thomas.bruyelle@gmail.com>
Co-authored-by: jaekwon <jae@tendermint.com>
thehowl added a commit that referenced this issue Mar 18, 2024
Split from #1695 for ease of reviewing. Related to #814. Merge order:

1. #1700 (this one!) 
2. #1702
3. #1695

I scrapped the "linked identifier" feature of native bindings. The
reasoning mostly stems from the changes subsequently implemented in
#1695, as having "linked identifiers" means that:

- the Gno linked types must have a different name from Go's if they are
within the same package, so their names don't conflict after
transpilation
- in order for the "linked types" to match in the generated code, the
AST has to be rewritten to make a type alias (ie. `type Address =
crypto.Bech32Address`)
- while still allowing the type to be "modifiable" by the std package,
because we want to add our own methods -- this is not possible for
imported types, obviously
- and if we try removing methods, this [creates
errors](https://drop.howl.moe/9ba0d53115-Screenshot_from_2024-02-27_23-50-34.png)
because the methods on the original types don't match 1-1 those
implemented in AST

Although, I think a decent case can be made besides easing our (my) life
in transpiling:

- Under the hood, the linked type mechanism works with
`Store.AddGno2GoMapping`. This uses gonative (read: the bane of my
existence -- #1361). If we remove all linked types, we can still pass
data between Go and Gno using type literals, which don't incur in naming
conflicts.
- This also makes the workings of "native bindings" clearer in general,
as they don't have any special linked types (which were currently
hardcoded in the misc/genstd source)

## Reviewing notes

- `Banker` got severely refactored as it was mapping entire go
interfaces into Go; it now uses simple elementary functions, with its
old behaviour split between Go and Gno.
- many other functions (std/native.gno) have also been changed so that
their native function only uses primitive types (so everything that used
an Address, now uses a string).
- Due to the naming conflicts already mentioned, Go's Banker has been
changed to `BankerInterface` so that it doesn't conflict.
- AddGo2GnoMapping is unused in the codebase. This has been removed from
Store to disencourage any further usage; removal of Go2Gno code is out
of scope for this PR (see #1361)
thehowl added a commit that referenced this issue Jun 19, 2024
Merge order:

1. #1700 
2. #1702
3. #1695 (this one!) -- review earlier ones first, if they're still
open!

This PR modifies the Gno transpiler (fka precompiler) to use Gno's
standard libraries rather than Go's when performing transpilation. This
creates the necessity to transpile Gno standard libraries, and as such
support their native bindings. And it removes the necessity for a
package like `stdshim`, and a mechanism like `stdlibWhitelist`.

- Fixes #668. Fixes #1865.
- Resolves #892.
- Part of #814. 
- Makes #1475 / #1576 possible without using hacks like `stdshim`.

cc/ @leohhhn @tbruyelle, as this relates to your work

## Why?

- This PR enables us to perform Go type-checking across the board, and
not use Go's standard libraries in transpiled code. This enables us to
_properly support our own standard libraries_, such as `std` but any
others we might want or need.
- It also paves the way further to go full circle, and have Gno code be
transpiled to Go, and then have "compilable" gno code

## Summary of changes

- The transpiler has been thoroughly refactored.
- The biggest change is described above: instead of maintaing the import
paths like `"strconv"` and `"math"` the same (so using Gno's stdlibs in
Gno, and Go's in Go), the import paths for standard libraries is now
also updated to point to the Gno standard libraries.
- Native functions are handled by removing their definitions when
transpiling, and changing their call expressions where appropriate. This
links the transpiled code directly to their native counterparts.
  - This removes the necessity for `stdlibWhitelist`. 
- As a consequence, `stdshim` is no longer needed and has been removed.
- Test files are still not "strictly checked": they may reference
stdlibs with no matching source, and will not be tested when running
with `--gobuild`. This is because packages like `fmt` have no
representation in Gno code; they only exist as injections in
`tests/imports.go`. I'll fix this eventually :)
- The CLI (`gno transpile`) has been changed to reflect the above
changes.
- Flag `--skip-fmt` has been removed (the result of transpile is always
formatted, anyway), and `--gofmt-binary` too, obviously. `gno transpile`
does not perform validation, but will gladly provide helpful validation
with the `--gobuild` flag.
- There is another PR that adds type checking in `gno lint`, without
needing to run through the transpilation step first:
#1730
- It now works by default by looking at "packages" rather than
individual files. This is necessary so that when performing `transpile`
on the `examples` directory, we can skip those where the gno.mod marks
the module as draft. These modules make use of packages like "fmt",
which because they don't have an underlying gno/go source, cannot be
transpiled.
- Running with `-gobuild` now handles more errors correctly; ie., all
errors not previously captured by the `errorRe` which only matches those
pertaining to a specific file/line.
  - `gnoFilesFromArgs` was unused and as such deleted
- `gnomod`'s behaviour was slightly changed.
- I am of the opinion that `gno mod download` should not precompile what
it downloads; _especially_ to gather the dependencies it has. I've
changed it so that it does a `OnlyImports` parse of the file it
downloads to fetch additional dependencies

Misc:

- `Makefile` now contains a recipe to calculate the coverage for
`gnovm/cmd/gno`, and also view it via the HTML interface. This is needed
as it has a few extra steps (which @gfanton already previously added in
the CI).
- Realms r/demo/art/gnoface and r/x/manfred_outfmt have been marked as
draft, as they depend on packages which are not actually present in the
Gno standard libraries.
  - The transpiler now ignores draft packages by default.
- `ReadMemPackage` now also considers Go files. This is meant to have
on-chain the code for standard libraries like `std` which have native
bindings. We still exclude Go code if it's not in a standard library.
- `//go:build` constraints have been removed from standard libraries, as
go files can only have one and we already add our own when transpiling

## Further improvements

after this PR

- Scope understanding in `transpiler` (so call expressions are not
incorrectly rewritten)
- Correctly transpile gno.mod

---------

Co-authored-by: Antonio Navarro Perez <antnavper@gmail.com>
Co-authored-by: Miloš Živković <milos.zivkovic@tendermint.com>
gfanton pushed a commit to gfanton/gno that referenced this issue Jul 23, 2024
Merge order:

1. gnolang#1700 
2. gnolang#1702
3. gnolang#1695 (this one!) -- review earlier ones first, if they're still
open!

This PR modifies the Gno transpiler (fka precompiler) to use Gno's
standard libraries rather than Go's when performing transpilation. This
creates the necessity to transpile Gno standard libraries, and as such
support their native bindings. And it removes the necessity for a
package like `stdshim`, and a mechanism like `stdlibWhitelist`.

- Fixes gnolang#668. Fixes gnolang#1865.
- Resolves gnolang#892.
- Part of gnolang#814. 
- Makes gnolang#1475 / gnolang#1576 possible without using hacks like `stdshim`.

cc/ @leohhhn @tbruyelle, as this relates to your work

## Why?

- This PR enables us to perform Go type-checking across the board, and
not use Go's standard libraries in transpiled code. This enables us to
_properly support our own standard libraries_, such as `std` but any
others we might want or need.
- It also paves the way further to go full circle, and have Gno code be
transpiled to Go, and then have "compilable" gno code

## Summary of changes

- The transpiler has been thoroughly refactored.
- The biggest change is described above: instead of maintaing the import
paths like `"strconv"` and `"math"` the same (so using Gno's stdlibs in
Gno, and Go's in Go), the import paths for standard libraries is now
also updated to point to the Gno standard libraries.
- Native functions are handled by removing their definitions when
transpiling, and changing their call expressions where appropriate. This
links the transpiled code directly to their native counterparts.
  - This removes the necessity for `stdlibWhitelist`. 
- As a consequence, `stdshim` is no longer needed and has been removed.
- Test files are still not "strictly checked": they may reference
stdlibs with no matching source, and will not be tested when running
with `--gobuild`. This is because packages like `fmt` have no
representation in Gno code; they only exist as injections in
`tests/imports.go`. I'll fix this eventually :)
- The CLI (`gno transpile`) has been changed to reflect the above
changes.
- Flag `--skip-fmt` has been removed (the result of transpile is always
formatted, anyway), and `--gofmt-binary` too, obviously. `gno transpile`
does not perform validation, but will gladly provide helpful validation
with the `--gobuild` flag.
- There is another PR that adds type checking in `gno lint`, without
needing to run through the transpilation step first:
gnolang#1730
- It now works by default by looking at "packages" rather than
individual files. This is necessary so that when performing `transpile`
on the `examples` directory, we can skip those where the gno.mod marks
the module as draft. These modules make use of packages like "fmt",
which because they don't have an underlying gno/go source, cannot be
transpiled.
- Running with `-gobuild` now handles more errors correctly; ie., all
errors not previously captured by the `errorRe` which only matches those
pertaining to a specific file/line.
  - `gnoFilesFromArgs` was unused and as such deleted
- `gnomod`'s behaviour was slightly changed.
- I am of the opinion that `gno mod download` should not precompile what
it downloads; _especially_ to gather the dependencies it has. I've
changed it so that it does a `OnlyImports` parse of the file it
downloads to fetch additional dependencies

Misc:

- `Makefile` now contains a recipe to calculate the coverage for
`gnovm/cmd/gno`, and also view it via the HTML interface. This is needed
as it has a few extra steps (which @gfanton already previously added in
the CI).
- Realms r/demo/art/gnoface and r/x/manfred_outfmt have been marked as
draft, as they depend on packages which are not actually present in the
Gno standard libraries.
  - The transpiler now ignores draft packages by default.
- `ReadMemPackage` now also considers Go files. This is meant to have
on-chain the code for standard libraries like `std` which have native
bindings. We still exclude Go code if it's not in a standard library.
- `//go:build` constraints have been removed from standard libraries, as
go files can only have one and we already add our own when transpiling

## Further improvements

after this PR

- Scope understanding in `transpiler` (so call expressions are not
incorrectly rewritten)
- Correctly transpile gno.mod

---------

Co-authored-by: Antonio Navarro Perez <antnavper@gmail.com>
Co-authored-by: Miloš Živković <milos.zivkovic@tendermint.com>
@linear linear bot removed 🌟 improvement performance improvements, refactors ... 📦 🤖 gnovm Issues or PRs gnovm related labels Sep 11, 2024
thehowl added a commit that referenced this issue Oct 17, 2024
This PR adds the full `strconv` package, implemented as pure Gno code.
It removes the native functions `Itoa` (and others), and adds support
for new functions such as `FormatFloat`.

# Summary of changes

- Standard libraries
- Changes of `strconv` from Go stdlib:
https://gist.github.com/thehowl/904b42b1ea53fef9b8a0486155c02b73 --
tldr:
    - removed everything related to complex types.
    - avoid using reflection and dot imports.
- instead of loading testfp.txt, embed the file directly into the
source.
    - define min/max explicitly as they're not built-in yet.
    - remove go:build tags.
    - (all of these mostly involve test files.)
  - `unicode`
    - Update tables, so that `strconv` tests succeed.
  - `unicode/utf8`
- Update to latest go version. Mostly, use `fallback` (as we now
half-support it) and use `AppendString`.
- GnoVM
- PackageInjector is no longer necessary (hallelujah), see #814 for
context. This justifies the changes in `store.go`, `store_test.go`,
`nodes.go`, `tests/imports.go`.
  - `gonative.go` and `machine.go` changes improve some error messages.
- `preprocess.go` changes fix a bug which can be seen in the `for20.gno`
test. If a `for` loop is labeled, then a bare `break` (ie. without a
label to break to) would panic, as it wouldn't find any for loop without
a label (in `findBranchLabel`). I added a regression test as well as a
couple test showing the error message for when we misplace
continue/break statements.
- Tests.
- `strconv.Itoa` now uses more gas than its existing native
implementation. This is to be expected; we can consider moving it back
to a native implementation if we deem it useful for performance, but I
think it's good for us to work on having as much code implemented
directly in gno before moving it back to Go for performance.

---------

Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🌟 Wanted for Launch
Status: In Progress
Development

No branches or pull requests

5 participants