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

Allow registering immutable struct types that will not be deep-copied #7

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ziemekobel-ef
Copy link

Adds RegisterImmutableType(t Type) that registers a type as immutable. This means that when a deep copy is made, if the type of the value being copied is the same as the type passed in, the value will not be copied. Instead, the original value will be used. This is useful for types that are immutable

Fixes: #6 assuming that the library user adds:

RegisterImmutableType(reflect.TypeOf(time.Time{}))

@ziemekobel-ef ziemekobel-ef changed the title Allows registering immutable struct types that will not be deep-copied Allow registering immutable struct types that will not be deep-copied Apr 17, 2023
@BenjamenMeyer
Copy link

BenjamenMeyer commented Dec 22, 2023

Question: it is that it's immutable? Or that the type does not export internal data? Just because a type does not expose any public value members does not mean that it is immutable.

Per #6 see https://cs.opensource.google/go/go/+/refs/tags/go1.21.5:src/time/time.go;l=129-150 - the value members are all private data members.

The Time type (per issue #6) does not export anything. If go-deepcopy is relying on public members for the copy then that would be why #6 is failing. However, this may expose a deeper issue with go-deepcopy if it cannot get types to copy their private members as well.

FWIW - the encoding/json package that Go provides has does do some oddball things to try to get private members to encode (see https://cs.opensource.google/go/go/+/refs/tags/go1.21.5:src/encoding/json/decode.go;l=426-434).

NOTE: I ran into a similar issue with my own struct; I do want a copy proper copy but Go makes that kind of hard given the public/private limitations of encoding systems as the reflect functionality does not expose the private members.

@ziemekobel-ef
Copy link
Author

@BenjamenMeyer time.Time is immutable, so this PR does fix #6

But, yes, you are right that it will not work for other types that are mutable, but do not expose private members.

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.

time.Time problem
2 participants