search

mohae / deepcopy

Deep copy things
MIT License
569 stars 122 forks source link

Map keys aren't deep-copied #9

Closed divVerent closed 7 years ago

divVerent commented 7 years ago

Map keys are shallow-copied here: https://github.com/mohae/deepcopy/blob/master/deepcopy.go#L103

This should normally not affect any real world code as slices/maps are forbidden in map keys and pointers have no "one true correct handling" for map keys: if one deep-copies them, lookups by the original pointers won't match anymore, and if one shallow-copies them, code modifying the copy may change the original.

I suggest explicitly detecting pointers inside map keys (recursively, as they could be struct fields) and panicking when attempting to deep-copy such values. The detection result could be cached by the map key type to prevent unnecessary processing.

At the very least though, this should be documented.

mohae commented 7 years ago

Thanks for pointing this out.

The pointers inside a map should be detected and deep copied.

mohae commented 7 years ago

fixed with https://github.com/mohae/deepcopy/commit/d86b0a3b6606f464f61cbfdaddc7cd5ca7a169e0

divVerent commented 7 years ago

This doesn't handle the case of map keys being a struct with pointer inside though.

On Jun 2, 2017 15:01, "Joel Scoble" notifications@github.com wrote:

fixed with d86b0a3 https://github.com/mohae/deepcopy/commit/d86b0a3b6606f464f61cbfdaddc7cd5ca7a169e0

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mohae/deepcopy/issues/9#issuecomment-305881952, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPWsDan8m6PhxWv5N0ibMgjuZXeMkq_ks5sAFv3gaJpZM4NbaIE .

mohae commented 7 years ago

Thanks for pointing out the flaw in my thinking; I didn't think about the problem properly.

I've made it so that the key is always deep copied; hopefully this fully addresses this issue and I didn't miss something else!

I appreciate your feedback.