Change object types to be non-pointers during codegen #136
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is good. We can now trivially copy structs generated by pkl-gen-go (assuming no nullable properties). Any timeline for when this will be merged? |
Normal go conventions encourage the returning on structs rather than pointers or interfaces. It's also normal convension to make optional values pointers, and non-optional concrete types, including stucts, rather than pointers to structs. As it very easy for a someone to choose to take a pointer to a struct in go and pass that around if they want to avoid copies (although the go compiler is heavily optimised to support passing complete structs, even large ones, around), there's little benifit in making embeded structs pointers. It just requires that downstream code needs to be filled with annoying nil checkes, and generally makes it harder to write robust, clean code.
Codegen changes means that the old test fixtures are out of date, and can't be used to test the go binding code. Updating these fixtures breaks the binding code, and identifies the areas that need enhancement.
The codegen changes means we need to support decoding data into concrete structs, and not just pointers to structs.
* Revert rename-based changes (no I-prefix for interfaces, structs have Impl postfix) * Change built-in types to be non-pointer types too (pkl.Duration vs *pkl.Duration) * Detect whether `RegisterMapping` receives a pointer vs non-pointer (this is a breaking change) * Change `GeneratorSettings.pkl.go` to use non-pointer types
bioball
force-pushed
the
concrete-structs
branch
from
e480624 to
5ac4228
Compare
Member
Author
|
@jalius this is merged, and we'll aim to release a new version of pkl-go sometime in the next two weeks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thanks to @thomaspurchas for their contributions here!
This is based on top of #92.
Closes #92
This pull request changes the Go code generator to produce non-pointer types for nullables.
The problem this aims to solve is to address an issue where pointers are used both to represent values that are nullable, and values that are objects. This causes ambiguity (did this come from a nullable type in Pkl?)
Users that prefer the old style for whatever reason can still use an older version of
pkl-gen-go.The decoder determines the correct type based on whether the declared type is a pointer or not.