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

refactor(go): stop emitting unused fields in go structs #2612

Merged
merged 1 commit into from
Feb 28, 2021

Conversation

RomainMuller
Copy link
Contributor

@RomainMuller RomainMuller commented Feb 25, 2021

Go struct fields used to be emitted for all properties of any type,
including for go structs merely there to act as a proxy to a JS object.
Those properties were never accessed (neither set not read), and could
have been cause for confusion. They additionally needlessly increased
the memory footprint of such data types.

This changes the code generation to no longer emit fields on go structs
that are intended as proxies to JS objects, instead embedding the base
type(s) (super-class and/or super-interfaces) to properly inherit their
methods without duplicating every declaration; and uses a byte
placeholder when needed to ensure no 0-width struct is ever generated
(go would have those take 0 bytes of memory, and as a consequence, any
such vlaue is deemed equal to any other, which is not what we want).

In cases when a type descends from more than one direct parent, and
any of their (transitive) bases belongs to a different assembly, all members,
including inherited, are re-implemented to avoid the risk of a conflict
occurring if two parents bring in the same declaration (then automated
promotion cannot happen anymore).

This implied making a change to the method registration functions, such
that each type (class and interface) registers a proxy maker
function that is used by the runtime library to properly initialize jsii
proxy instances of objects without requiring user intervention. This
makes it safe to operate even when embedding data structures from other
libraries. Additionally, it enables hiding the JS proxy structs from the
exported APIs, which further reduces the risk of mis-use.

These changes were proposed in aws/aws-cdk-rfcs#292.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Go struct fields used to be emitted for all properties of any type,
including for go structs merely there to act as a proxy to a JS object.
Those properties were never accessed (neither set not read), and could
have been cause for confusion. They additionally needlessly increased
the memory footprint of such data types.

This changes the code generation to no longer emit fields on go structs
that are intended as proxies to JS objects, instead embedding the base
type(s) (super-class and/or super-interfaces) to properly inherit their
methods without duplicating every declaration; and uses a `byte`
placeholder when needed to ensure no 0-width struct is ever generated
(go would have those take 0 bytes of memory, and as a consequence, any
such vlaue is deemed equal to any other, which is not what we want).

In cases when a member is inherited from more than one parent, that
member is re-declared on the descendant type, as otherwise the promotion
is ambiguous.

This implied making a change to the method registration functions, such
that each type (class and interface) registers a proxy `maker`
function that is used by the runtime library to properly initialize jsii
proxy instances  of objects without requiring user intervention. This
makes it safe to operate even when embedding data structures from other
libraries. Additionally, it enables hiding the JS proxy structs from the
exported APIs, which further reduces the risk of mis-use.
@RomainMuller RomainMuller added language/go Regarding GoLang bindings effort/medium Medium work item – a couple days of effort module/pacmak Issues affecting the `jsii-pacmak` module module/runtime Issues affecting the `jsii-runtime` contribution/core This is a PR that came from AWS. labels Feb 25, 2021
@RomainMuller RomainMuller requested a review from a team February 25, 2021 17:59
@RomainMuller RomainMuller self-assigned this Feb 25, 2021
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-6Jw05QLuWWwe
  • Commit ID: 26882ef
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Contributor

@MrArnoldPalmer MrArnoldPalmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a meaty one 🥩 . Can't find anything to complain about. Snapshots look good and codegen feels simpler without the GoStruct type from go-type and/or the need to deal with name vs interfaceName in type-ref. 👍🏻

@eladb eladb merged commit adfe8b9 into main Feb 28, 2021
@eladb eladb deleted the rmuller/go-redundant-struct-fields branch February 28, 2021 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. effort/medium Medium work item – a couple days of effort language/go Regarding GoLang bindings module/pacmak Issues affecting the `jsii-pacmak` module module/runtime Issues affecting the `jsii-runtime`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants