Skip to content

Commit

Permalink
BEGIN_PUBLIC
Browse files Browse the repository at this point in the history
Allow StarlarkValue#str to access StarlarkSemantics

This allows us to alter the behavior of `str(some_starlark_value)` depending on flags. See child CL for an example.

This involves adding the StarlarkSemantics argument to a rather long list of call sites and implementors, but it helps in the end to clarify that stringification is really dependent on starlark semantics.
END_PUBLIC

Work towards #15916

PiperOrigin-RevId: 462564920
Change-Id: I1733424a5657e54cb19f117b8b5dc7af76ca0842
  • Loading branch information
Wyverald authored and copybara-github committed Jul 22, 2022
1 parent f069347 commit 6751724
Show file tree
Hide file tree
Showing 28 changed files with 142 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkSemantics;

/**
* A {@link com.google.devtools.build.lib.analysis.ConfiguredTarget} that is produced by a rule.
Expand Down Expand Up @@ -179,8 +180,9 @@ public <P extends TransitiveInfoProvider> P getProvider(Class<P> providerClass)

@Override
public String getErrorMessageForUnknownField(String name) {
return Starlark.format(
"%r (rule '%s') doesn't have provider '%s'", this, getRuleClassString(), name);
return String.format(
"%s (rule '%s') doesn't have provider '%s'",
Starlark.repr(this), getRuleClassString(), name);
}

@Override
Expand Down Expand Up @@ -218,7 +220,7 @@ public void repr(Printer printer) {
}

@Override
public void debugPrint(Printer printer) {
public void debugPrint(Printer printer, StarlarkSemantics semantics) {
// Show the names of the provider keys that this target propagates.
// Provider key names might potentially be *private* information, and thus a comprehensive
// list of provider keys should not be exposed in any way other than for debug information.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalE
// It's easier to use the Starlark repr as a string form, not what AutoValue produces.
@Override
public final String toString() {
return Starlark.str(this);
return Starlark.repr(this);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,9 @@ public int hashCode() {

@Override
public void repr(Printer printer) {
Printer.format(printer, "ConstraintSettingInfo(%s", label.toString());
printer.append("ConstraintSettingInfo(").append(label.toString());
if (defaultConstraintValueLabel != null) {
Printer.format(
printer, ", default_constraint_value=%s", defaultConstraintValueLabel.toString());
printer.append(", default_constraint_value=").append(defaultConstraintValueLabel.toString());
}
printer.append(")");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,7 @@ public ConfigMatchingProvider configMatchingProvider(PlatformInfo platformInfo)

@Override
public void repr(Printer printer) {
Printer.format(
printer,
"ConstraintValueInfo(setting=%s, %s)",
constraint.label().toString(),
label.toString());
printer.append(String.format("ConstraintValueInfo(setting=%s, %s)", constraint.label(), label));
}

/** Returns a new {@link ConstraintValueInfo} with the given data. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ public ImmutableMap<String, String> execProperties() {

@Override
public void repr(Printer printer) {
Printer.format(
printer, "PlatformInfo(%s, constraints=%s)", label.toString(), constraints.toString());
printer.append(String.format("PlatformInfo(%s, constraints=%s)", label, constraints));
}

/** Returns a new {@link Builder} for creating a fresh {@link PlatformInfo} instance. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public Label typeLabel() {

@Override
public void repr(Printer printer) {
Printer.format(printer, "ToolchainTypeInfo(%s)", typeLabel);
printer.append(String.format("ToolchainTypeInfo(%s)", typeLabel));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public void repr(Printer printer) {
}

@Override
public void debugPrint(Printer printer) {
public void debugPrint(Printer printer, StarlarkSemantics semantics) {
try {
printer.append(Joiner.on(" ").join(build().arguments()));
} catch (CommandLineExpansionException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,9 @@ private static ResolvedEvent resolveBind(Rule rule) {
String name = rule.getName();
Label actual = (Label) rule.getAttr("actual");
String nativeCommand =
Starlark.format("bind(name = %r, actual = %r)", name, actual.getCanonicalForm());
String.format(
"bind(name = %s, actual = %s)",
Starlark.repr(name), Starlark.repr(actual.getCanonicalForm()));

return new ResolvedEvent() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ public void repr(Printer printer) {
}

@Override
public void str(Printer printer) {
public void str(Printer printer, StarlarkSemantics semantics) {
printer.append(getCanonicalForm());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ public Map<Label, ValueT> convert(Object x, Object what, LabelConverter labelCon
errorMessage.append(',');
}
errorMessage.append(' ');
errorMessage.str(entry.getKey());
errorMessage.append(entry.getKey().getCanonicalForm());
errorMessage.append(" (as ");
errorMessage.repr(entry.getValue());
errorMessage.append(')');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
Starlark.fastcall(
thread, this.formatFn, new Object[] {target.getConfiguredTarget()}, NO_ARGS);

addResult(Starlark.str(result));
addResult(Starlark.str(result, thread.getSemantics()));
} catch (EvalException ex) {
eventHandler.handle(
Event.error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,23 +275,23 @@ public Sequence<Linkstamp> getLinkstampsForStarlark() {
}

@Override
public void debugPrint(Printer printer) {
public void debugPrint(Printer printer, StarlarkSemantics semantics) {
printer.append("<LinkerInput(owner=");
if (owner == null) {
printer.append("[null owner, uses old create_linking_context API]");
} else {
owner.debugPrint(printer);
owner.debugPrint(printer, semantics);
}
printer.append(", libraries=[");
for (LibraryToLink libraryToLink : libraries) {
libraryToLink.debugPrint(printer);
libraryToLink.debugPrint(printer, semantics);
printer.append(", ");
}
printer.append("], userLinkFlags=[");
printer.append(Joiner.on(", ").join(userLinkFlags));
printer.append("], nonCodeInputs=[");
for (Artifact nonCodeInput : nonCodeInputs) {
nonCodeInput.debugPrint(printer);
nonCodeInput.debugPrint(printer, semantics);
printer.append(", ");
}
// TODO(cparsons): Add debug repesentation of linkstamps.
Expand Down Expand Up @@ -396,10 +396,10 @@ public String toString() {
@Nullable private final ExtraLinkTimeLibraries extraLinkTimeLibraries;

@Override
public void debugPrint(Printer printer) {
public void debugPrint(Printer printer, StarlarkSemantics semantics) {
printer.append("<CcLinkingContext([");
for (LinkerInput linkerInput : linkerInputs.toList()) {
linkerInput.debugPrint(printer);
linkerInput.debugPrint(printer, semantics);
printer.append(", ");
}
printer.append("])>");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.FeatureConfigurationApi;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.StarlarkSemantics;

/**
* Wrapper for {@link FeatureConfiguration}, {@link CppConfiguration}, and {@link BuildOptions}.
Expand Down Expand Up @@ -58,7 +59,7 @@ public FeatureConfiguration getFeatureConfiguration() {
}

@Override
public void str(Printer printer) {
public void str(Printer printer, StarlarkSemantics semantics) {
printer.append("<FeatureConfiguration(");
printer.append("ENABLED:");
printer.append(Joiner.on(", ").join(featureConfiguration.getEnabledFeatureNames()));
Expand All @@ -68,7 +69,7 @@ public void str(Printer printer) {
}

@Override
public void debugPrint(Printer printer) {
public void debugPrint(Printer printer, StarlarkSemantics semantics) {
printer.append("<FeatureConfiguration(");
printer.append(Joiner.on(", ").join(featureConfiguration.getEnabledFeatureNames()));
printer.append(")>");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.StarlarkList;
import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.StarlarkThread;

/** Encapsulates information for linking a library. */
Expand Down Expand Up @@ -226,7 +227,7 @@ LinkerInputs.LibraryToLink getInterfaceLibraryToLink() {
abstract boolean getDisableWholeArchive();

@Override
public final void debugPrint(Printer printer) {
public final void debugPrint(Printer printer, StarlarkSemantics semantics) {
printer.append("<LibraryToLink(");
printer.append(
Joiner.on(", ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ private static ResolvedEvent resolve(Rule rule, BlazeDirectories directories) {
} else {
pathArg = Starlark.repr(path);
}
String repr = Starlark.format("local_repository(name = %r, path = %s)", name, pathArg);
String repr =
String.format("local_repository(name = %s, path = %s)", Starlark.repr(name), pathArg);
return new ResolvedEvent() {
@Override
public String getName() {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/net/starlark/java/eval/EvalUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,9 @@ static Object binaryOp(TokenKind op, Object x, Object y, StarlarkThread starlark
String xs = (String) x;
try {
if (y instanceof Tuple) {
return Starlark.formatWithList(xs, (Tuple) y);
return Starlark.formatWithList(semantics, xs, (Tuple) y);
} else {
return Starlark.format(xs, y);
return Starlark.format(semantics, xs, y);
}
} catch (IllegalFormatException ex) {
throw new EvalException(ex);
Expand Down
11 changes: 7 additions & 4 deletions src/main/java/net/starlark/java/eval/FormatParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ final class FormatParser {
* @param kwargs Named arguments
* @return The formatted string
*/
String format(String input, List<Object> args, Map<String, Object> kwargs) throws EvalException {
String format(
String input, List<Object> args, Map<String, Object> kwargs, StarlarkSemantics semantics)
throws EvalException {
char[] chars = input.toCharArray();
StringBuilder output = new StringBuilder();
History history = new History();
Expand All @@ -56,7 +58,7 @@ String format(String input, List<Object> args, Map<String, Object> kwargs) throw
int advancePos = 0;

if (current == '{') {
advancePos = processOpeningBrace(chars, pos, args, kwargs, history, output);
advancePos = processOpeningBrace(chars, pos, args, kwargs, history, output, semantics);
} else if (current == '}') {
advancePos = processClosingBrace(chars, pos, output);
} else {
Expand Down Expand Up @@ -88,7 +90,8 @@ private int processOpeningBrace(
List<Object> args,
Map<String, Object> kwargs,
History history,
StringBuilder output)
StringBuilder output,
StarlarkSemantics semantics)
throws EvalException {
Printer printer = new Printer(output);
if (has(chars, pos + 1, '{')) {
Expand Down Expand Up @@ -120,7 +123,7 @@ private int processOpeningBrace(
}

// Format object for output
printer.str(value);
printer.str(value, semantics);

// Advances the current position to the index of the closing brace of the
// replacement field. Due to the definition of the enclosing for() loop,
Expand Down
19 changes: 11 additions & 8 deletions src/main/java/net/starlark/java/eval/MethodLibrary.java
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,10 @@ public int len(Object x, StarlarkThread thread) throws EvalException {
"Converts any object to string. This is useful for debugging."
+ "<pre class=\"language-python\">str(\"ab\") == \"ab\"\n"
+ "str(8) == \"8\"</pre>",
parameters = {@Param(name = "x", doc = "The object to convert.")})
public String str(Object x) throws EvalException {
return Starlark.str(x);
parameters = {@Param(name = "x", doc = "The object to convert.")},
useStarlarkThread = true)
public String str(Object x, StarlarkThread thread) throws EvalException {
return Starlark.str(x, thread.getSemantics());
}

@StarlarkMethod(
Expand Down Expand Up @@ -679,15 +680,17 @@ public StarlarkList<?> dir(Object object, StarlarkThread thread) throws EvalExce
name = "args",
doc =
"A list of values, formatted with str and joined with spaces, that appear in the"
+ " error message."))
public void fail(Object msg, Object attr, Tuple args) throws EvalException {
+ " error message."),
useStarlarkThread = true)
public void fail(Object msg, Object attr, Tuple args, StarlarkThread thread)
throws EvalException {
List<String> elems = new ArrayList<>();
// msg acts like a leading element of args.
if (msg != Starlark.NONE) {
elems.add(Starlark.str(msg));
elems.add(Starlark.str(msg, thread.getSemantics()));
}
for (Object arg : args) {
elems.add(Starlark.str(arg));
elems.add(Starlark.str(arg, thread.getSemantics()));
}
String str = Joiner.on(" ").join(elems);
if (attr != Starlark.NONE) {
Expand Down Expand Up @@ -724,7 +727,7 @@ public void print(String sep, Sequence<?> args, StarlarkThread thread) throws Ev
String separator = "";
for (Object x : args) {
p.append(separator);
p.debugPrint(x);
p.debugPrint(x, thread.getSemantics());
separator = sep;
}
// The PRINT_TEST_MARKER key is used in tests to verify the effects of command-line options.
Expand Down
24 changes: 13 additions & 11 deletions src/main/java/net/starlark/java/eval/Printer.java
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,13 @@ public final String toString() {
*
* <p>Implementations of StarlarkValue may define their own behavior of {@code debugPrint}.
*/
public Printer debugPrint(Object o) {
public Printer debugPrint(Object o, StarlarkSemantics semantics) {
if (o instanceof StarlarkValue) {
((StarlarkValue) o).debugPrint(this);
((StarlarkValue) o).debugPrint(this, semantics);
return this;
}

return this.str(o);
return this.str(o, semantics);
}

/**
Expand All @@ -130,12 +130,12 @@ public Printer debugPrint(Object o) {
*
* <p>Implementations of StarlarkValue may define their own behavior of {@code str}.
*/
public Printer str(Object o) {
public Printer str(Object o, StarlarkSemantics semantics) {
if (o instanceof String) {
return this.append((String) o);

} else if (o instanceof StarlarkValue) {
((StarlarkValue) o).str(this);
((StarlarkValue) o).str(this, semantics);
return this;

} else {
Expand Down Expand Up @@ -299,13 +299,15 @@ private void pop() {
* @param arguments an array containing arguments to substitute into the format operators in order
* @throws IllegalFormatException if the format string is invalid or the arguments do not match it
*/
public static void format(Printer printer, String format, Object... arguments) {
formatWithList(printer, format, Arrays.asList(arguments));
public static void format(
Printer printer, StarlarkSemantics semantics, String format, Object... arguments) {
formatWithList(printer, semantics, format, Arrays.asList(arguments));
}

/** Same as {@link #format}, but with a list instead of variadic args. */
@SuppressWarnings("FormatString") // see b/178189609
public static void formatWithList(Printer printer, String pattern, List<?> arguments) {
public static void formatWithList(
Printer printer, StarlarkSemantics semantics, String pattern, List<?> arguments) {
// N.B. MissingFormatWidthException is the only kind of IllegalFormatException
// whose constructor can take and display arbitrary error message, hence its use below.
// TODO(adonovan): this suggests we're using the wrong exception. Throw IAE?
Expand Down Expand Up @@ -370,7 +372,7 @@ public static void formatWithList(Printer printer, String pattern, List<?> argum
String.format(
"got %s for '%%%c' format, want int or float", Starlark.type(arg), conv));
}
printer.str(
printer.append(
String.format(
conv == 'd' ? "%d" : conv == 'o' ? "%o" : conv == 'x' ? "%x" : "%X", n));
continue;
Expand All @@ -394,15 +396,15 @@ public static void formatWithList(Printer printer, String pattern, List<?> argum
String.format(
"got %s for '%%%c' format, want int or float", Starlark.type(arg), conv));
}
printer.str(StarlarkFloat.format(v, conv));
printer.append(StarlarkFloat.format(v, conv));
continue;

case 'r':
printer.repr(arg);
continue;

case 's':
printer.str(arg);
printer.str(arg, semantics);
continue;

default:
Expand Down
Loading

0 comments on commit 6751724

Please sign in to comment.