Skip to content

Commit

Permalink
Ensure **kwargs initialized by positionalOnlyCall is mutable
Browse files Browse the repository at this point in the history
The fix in cf94101 made an empty **kwargs immutable when initialized via
positionalOnlyCall. Unfortunately, that was an incompatible change (although
unlikely to affect anything in practice given how positionalOnlyCall is
currently used): the Starlark language spec does not specify whether **kwargs
is mutable, but **kwargs is mutable in Python 3 and was always mutable
previously in the Starlark interpreter.

See bazelbuild/starlark#295 - perhaps it ought to
be immutable in Starlark, but that requires a language spec change and an
incompatible flag.

PiperOrigin-RevId: 712920880
Change-Id: I151bb29e9646b82def8944143198f3dffb398583
  • Loading branch information
tetromino authored and copybara-github committed Jan 7, 2025
1 parent 92c8e71 commit eca313a
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
9 changes: 5 additions & 4 deletions src/main/java/net/starlark/java/eval/StarlarkFunction.java
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public Object positionalOnlyCall(StarlarkThread thread, Object... positional)
checkRecursive(thread);
ArgumentProcessor argumentProcessor = new ArgumentProcessor(this);
// Feed only positional arguments into the argument processor.
argumentProcessor.processPositionalOnly(positional);
argumentProcessor.processPositionalOnly(positional, thread.mutability());
return callWithArguments(thread, argumentProcessor);
}

Expand Down Expand Up @@ -392,13 +392,14 @@ void processPositionalAndNamed(Object[] positional, Object[] named, Mutability m
bindNamedArgsToLocals(named, mu);
}

void processPositionalOnly(Object[] positional) throws EvalException {
void processPositionalOnly(Object[] positional, Mutability mu) throws EvalException {
numNonSurplusPositionalArgs = getNumNonSurplusPositionalArgs(positional);
bindPositionalArgsToLocals(positional);
bindSurplusPositionalArgsToVarArgs(positional);
// Bind an empty dict to **kwargs if present.
// Bind an empty dict to **kwargs if present. (The dict, unfortunately, needs to be mutable;
// see https://github.com/bazelbuild/starlark/issues/295)
if (owner.rfn.hasKwargs()) {
locals[owner.rfn.getParameters().size() - 1] = Dict.empty();
locals[owner.rfn.getParameters().size() - 1] = Dict.of(mu);
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/test/java/net/starlark/java/eval/FunctionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -668,13 +668,14 @@ public void positionalOnlyCall_setsKeywordArgsVarargsAndKwargs() throws Exceptio
ev.exec(
"""
def f(a, b, *args, k = 42, **kwargs):
kwargs["mutable"] = True # verify that **kwargs is a mutable dict
return "k=%s args=%s kwargs=%s" % (repr(k), repr(args), repr(kwargs))
""");
StarlarkFunction f = (StarlarkFunction) ev.lookup("f");
try (Mutability mu = Mutability.create("test")) {
StarlarkThread thread = StarlarkThread.createTransient(mu, StarlarkSemantics.DEFAULT);
assertThat((String) Starlark.positionalOnlyCall(thread, f, "a", "b", "c", "d"))
.isEqualTo("k=42 args=(\"c\", \"d\") kwargs={}");
.isEqualTo("k=42 args=(\"c\", \"d\") kwargs={\"mutable\": True}");
}
}
}

0 comments on commit eca313a

Please sign in to comment.