Skip to content

[codegen] Generated Copy Code #424

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

[codegen] Generated Copy Code #424

wants to merge 7 commits into from

Conversation

IfSentient
Copy link
Contributor

Draft PR tracking work done a few months ago for copy codegen

IfSentient added a commit that referenced this pull request Apr 15, 2025
…#744)

`resource.CopyObject` and `resource.CopyObjectInto` incorrectly zero
`time.Time` values when doing their copy, as it tries to copy
`time.Time` instances as structs using reflection. Since `time.Time` has
no exported fields, this does not work. To fix this behavior, check if
the struct to be copied is `typeOf(time.Time)`, and, if so, directly
copy it with `dst.Set(src)`, rather than recursively calling
`CopyObjectInto` to copy it as a user-defined struct.

This does bring up the issue that any other "special" types which have
custom `MarshalJSON`/`UnmarshalJSON` implementations but no exported
fields will also fail with the deep copy performed by
`resource.CopyObject`/`resource.CopyObjectInto`. I'm not sure if this is
a restriction we want to live with, or attempt to solve here.

Ultimately, a better solution would be to avoid using relfection for
copying objects entirely, and rely on generated deep copy methods in
objects (see #424).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant