[chore] Remove NativeValue and NativeType from the VM entirely #1361
Assignees
Milestone
Comments
|
I'll probably take on this sometime in the next few months, even just as an experiment (or as part of sogno), even just to see the reduction in LOC :) |
This was referenced Feb 26, 2024
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)
7 tasks
This was referenced Oct 2, 2024
This was referenced Oct 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
This is nothing to work on yet, but rather we should wait until launch coding is almost complete, so that the codebase may settle a bit, such that the PR that removes NativeValue/NativeType can be referenced in the future for forks that want these things.
A prerequisite would be to get the code directory structure fixed, such as replacing gnovm/pkgs/gnolang with gnolang/pkgs/gnovm.
The text was updated successfully, but these errors were encountered: