Skip to content

Commit

Permalink
Starlark: dict.setdefault should fail if frozen
Browse files Browse the repository at this point in the history
... even if `setdefault` call did not update the dict.

This is similar to bazelbuild#12519.

Closes bazelbuild#12642.

RELNOTES: dict.setdefault(key, ...) now fails if dict is frozen, even if it already contains key. This is an incompatible API change.
PiperOrigin-RevId: 346093037
  • Loading branch information
stepancheg authored and copybara-github committed Dec 7, 2020
1 parent cb83aa8 commit b07c00c
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
15 changes: 6 additions & 9 deletions src/main/java/net/starlark/java/eval/Dict.java
Original file line number Diff line number Diff line change
Expand Up @@ -265,15 +265,12 @@ public Tuple popitem() throws EvalException {
named = true,
doc = "a default value if the key is absent."),
})
@SuppressWarnings("unchecked") // Cast of value to V
public Object setdefault(K key, Object defaultValue) throws EvalException {
// TODO(adonovan): opt: use putIfAbsent to avoid hashing twice.
Object value = get(key);
if (value != null) {
return value;
}
putEntry(key, (V) defaultValue);
return defaultValue;
public V setdefault(K key, V defaultValue) throws EvalException {
Starlark.checkMutable(this);
Starlark.checkHashable(key);

V prev = contents.putIfAbsent(key, defaultValue); // see class doc comment
return prev != null ? prev : defaultValue;
}

@StarlarkMethod(
Expand Down
4 changes: 4 additions & 0 deletions src/test/java/net/starlark/java/eval/testdata/dict.star
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ assert_eq(foo, bar)
assert_eq(foo.pop('a', 0), 0)
assert_eq(foo.popitem(), ('b', [1, 2]))

---
d = {1: 2}
freeze(d)
d.setdefault(1, 2) ### trying to mutate a frozen dict value
---
dict().popitem() ### empty dictionary
---
Expand Down

0 comments on commit b07c00c

Please sign in to comment.