Skip to content

Commit

Permalink
Fix StarlarkIndexable.getIndex implementations that return null.
Browse files Browse the repository at this point in the history
They should return Starlark.NONE instead.

Part of Optional Toolchains (bazelbuild#14726).

Closes bazelbuild#15344.

PiperOrigin-RevId: 444869683
  • Loading branch information
katre authored and copybara-github committed Apr 27, 2022
1 parent 9921a99 commit a48e246
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,12 @@ public Sequence<ConstraintSettingInfo> constraintSettings() {

@Override
public Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
return get(convertKey(key));
ConstraintSettingInfo constraintSettingInfo = convertKey(key);
Object result = get(constraintSettingInfo);
if (result == null) {
return Starlark.NONE;
}
return result;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.eval.StarlarkValue;

/**
* An implementation of ToolchainContextApi that can better handle converting strings into Labels.
Expand Down Expand Up @@ -99,7 +100,7 @@ private Label transformKey(StarlarkThread starlarkThread, Object key) throws Eva
}

@Override
public ToolchainInfo getIndex(
public StarlarkValue getIndex(
StarlarkThread starlarkThread, StarlarkSemantics semantics, Object key) throws EvalException {
Label toolchainTypeLabel = transformKey(starlarkThread, key);

Expand All @@ -116,7 +117,11 @@ public ToolchainInfo getIndex(
.map(Label::toString)
.collect(joining(", ")));
}
return toolchainContext().forToolchainType(toolchainTypeLabel);
ToolchainInfo toolchainInfo = toolchainContext().forToolchainType(toolchainTypeLabel);
if (toolchainInfo == null) {
return Starlark.NONE;
}
return toolchainInfo;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@
doc =
"Holds toolchains available for a particular exec group. Toolchain targets are accessed by"
+ " indexing with the toolchain type, as in"
+ " <code>context[\"//pkg:my_toolchain_type\"]</code>.")
+ " <code>ctx.toolchains[\"//pkg:my_toolchain_type\"]</code>. If the toolchain was"
+ " optional and no toolchain was resolved, this will return <code>None</code>.")
public interface ToolchainContextApi extends StarlarkValue, StarlarkIndexable.Threaded {}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.SkyKey;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkSemantics;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -244,16 +246,15 @@ public <T extends Info> T get(BuiltinProvider<T> provider) {
@Nullable
@Override
public Info get(Provider.Key providerKey) {

return null;
}

@Override
public void repr(Printer printer) {}

@Override
public Object getIndex(StarlarkSemantics semantics, Object key) {
return null;
public Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
throw Starlark.errorf("Unknown key '%s'", key);
}

@Override
Expand Down

0 comments on commit a48e246

Please sign in to comment.