bazelbuild / starlark

Starlark Language
Apache License 2.0
2.44k stars 163 forks source link

Add dict.copy/list.copy #192

Closed ndmitchell closed 3 years ago

ndmitchell commented 3 years ago

For dictionaries and lists, Python has x.copy() which creates a shallow copy of the dictionary/list. The way I've seen people doing this in Starlark is dict(**original) which is a much more complex operation both for the user, and for performance in implementation. Could these be added to Starlark?

stepancheg commented 3 years ago

dict(original) and list(original) work fine.

stepancheg commented 3 years ago

BTW, I noticed a pattern, people sometimes copy kwargs param not knowing that kwargs is a fresh dict, not shared with caller.

We may need some lint to point to this mistake.

ndmitchell commented 3 years ago

Thanks - excellent point! The Rust Starlark implementation also has copy. Do any of the other implementations? If not, it's probably best to delete these from the Rust variant.

I've also seen people cloning kwargs unnecessarily. It does seem a common theme.

ndmitchell commented 3 years ago

Thanks @stepancheg - withdrawing this suggestion, and I'll put it on my todo list to remove .copy from Rust Starlark.

carlosgalvezp commented 3 years ago

Hi,

This is not working, how to deep copy a dict when it contains lists inside?

def my_cc_binary(**kwargs_in):
    kwargs = dict(kwargs_in)
    kwargs["srcs"].append("baz.cpp")

SRCS = ["foo.cpp"]

my_cc_binary(name = "a", srcs = SRCS)

# Here, kwargs["srcs"] will contain "baz.cpp" even before it's added, 
# so it will be duplicated. SRCS has changed, but that was not intended!
my_cc_binary(name = "b", srcs = SRCS)