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

Primitive copy on maps #19

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

Conversation

enderv
Copy link

@enderv enderv commented Jan 19, 2022

Some lint fixes and adding an initial assignment to get primitives copied over.

With code I was working with some values were still empty. Tested on the struct I was having problems with and using a deepequal compare and all seems to be working now.

@@ -456,7 +459,11 @@ func walkType(source, sink, x string, m types.Type, w io.Writer, imports map[str
if b.Len() > 0 {
ksink = copyKSink
fmt.Fprintf(w, "var %s %s\n", ksink, kkind)
b.WriteTo(w)
fmt.Fprintf(w, "%s = %s\n", ksink, source)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi. could you tell me what the idea behind this line is? It looks like it would copy the pointer map to a new variable, but if that's the case I don't think we should be doing that, since we want copy the individual keys and values in the map itself.

Copy link
Author

Choose a reason for hiding this comment

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

thanks for the quick reply! Sorry for lack of initial explanation: What I found was on nested custom structs it would handle maps/slices ok but not any primitive types when it got to the bottom of the map. If I run deep-copy code for each individual type it would work ok but as we have a very complex type I did not want other devs to have to always remember to generate for each new one that (they might not even know its embedded).

for k6, v6 := range o.Meta.Data.CustomTypes {
    var cp_Meta_Data_CustomTypes_v6 CustomType
    if v6.MapField != nil {
        cp_Meta_Data_CustomTypes_v6.ArrayField = make(map[string]MapFieldType, len(v6.MapField))
       ...
    }
    if v6.MapField2 != nil {
        ....
    }
    ...continues on for all other complex types but misses strings and such.

With Change:
for k6, v6 := range o.Meta.Data.CustomTypes {
    var cp_Meta_Data_CustomTypes_v6 CustomType
    cp_Meta_Data_CustomTypes_v6 = v6 // will copy over everything first and then go on to complex types
    if v6.MapField != nil {
        cp_Meta_Data_CustomTypes_v6.ArrayField = make(map[string]MapFieldType, len(v6.MapField))
       ...
    }
    if v6.MapField2 != nil {
        ....
    }

Now that you called it out though I don't think this is working as expected either as if any pointer types it would not work correctly (just don't have any in our struct so missed it) will circle back

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like we'll need to somehow keep and satisfy the change in main_test.go:295, but discard the rest of the test changes and make sure that no other test cases are affected

@@ -469,7 +476,11 @@ func walkType(source, sink, x string, m types.Type, w io.Writer, imports map[str
if b.Len() > 0 {
vsink = copyVSink
fmt.Fprintf(w, "var %s %s\n", vsink, vkind)
b.WriteTo(w)
fmt.Fprintf(w, "%s = %s\n", vsink, val)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question as the above

@urandom
Copy link
Collaborator

urandom commented Jan 20, 2022

Do you happen to have a simplified case that illustrates the problem (like one of the issue_*.go files in testdata)?

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.

None yet

2 participants