[Bazel] using multable list as a default value in many of the rules
- Dominant language
- SystemVerilog
- Stars
- 3.6k
- Forks
- 1.1k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 141
Description
There are a number of bzl files with functions taking default values of `[]`. Which could be a source of unexpected behaviour, as (like Python) the default value objects are created when the function is defined, not called.
Which means if the function is called and the default value is used, then the default value object can be mutated inside the function or any function that value is then passed to. The next time the function is called, the default value will be the mutated value and not the expected empty list `[]`.
While this is a potential issue, it's not clear if the specific use case is present or not already. However I think it's worth fixing this up to avoid hitting this issue in the future.
See https://github.com/bazelbuild/starlark/blob/master/spec.md#functions
----
If a function parameter's default value is a mutable expression, modifications to the value during one call may be observed by subsequent calls. Beware of this when using lists or dicts as default values. If the function becomes frozen, its parameters' default values become frozen too.
```
# module a.sky
def f(x, list=[]):
list.append(x)
return list
f(4, [1,2,3]) # [1, 2, 3, 4]
f(1) # [1]
f(2) # [1, 2], not [2]!
# module b.sky
load("a.sky", "f")
f(3) # error: cannot append to frozen list
```
Contributor guide
Research direction
Search the repository's bzl files for functions using [] as default parameter values, then read the linked Starlark specification to confirm the behavior and identify cases that can be mutated. Review each affected rule and its callers; done means mutable defaults no longer risk sharing state between calls and the affected rules still behave correctly.
Written by the indexing model from the issue text.
Assessment
- Domain
- build-system
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100