Skip to content

Commit

Permalink
Remove deprecated legacy string representations of Skylark objects
Browse files Browse the repository at this point in the history
RELNOTES[INC]: The flag --incompatible_descriptive_string_representations is no
longer available, old style string representations of objects are not supported
anymore.

PiperOrigin-RevId: 171952621
  • Loading branch information
vladmos authored and hlopko committed Oct 13, 2017
1 parent ef143b4 commit cd6d8ae
Show file tree
Hide file tree
Showing 23 changed files with 21 additions and 356 deletions.
12 changes: 0 additions & 12 deletions site/docs/skylark/backward-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ guarded behind flags in the current release:
* [Dictionary literal has no duplicates](#dictionary-literal-has-no-duplicates)
* [New actions API](#new-actions-api)
* [Checked arithmetic](#checked-arithmetic)
* [Descriptive string representations](#descriptive-string-representations)

### Set constructor

Expand Down Expand Up @@ -291,15 +290,4 @@ All integers are stored using signed 32 bits.
* Default: `true`


### Descriptive string representations

For certain types of objects (such as `Label`, `File`, and rule contexts), this
flag changes the string representations returned by `str()` and `repr()` to be
more uniform and safe. In particular, the new representations are hermetic and
deterministic.

* Flag: `--incompatible_descriptive_string_representations`
* Default: `true`


<!-- Add new options here -->
Original file line number Diff line number Diff line change
Expand Up @@ -889,9 +889,4 @@ public void repr(SkylarkPrinter printer) {
printer.append("<generated file " + rootRelativePath + ">");
}
}

@Override
public void reprLegacy(SkylarkPrinter printer) {
printer.append(toString());
}
}
5 changes: 0 additions & 5 deletions src/main/java/com/google/devtools/build/lib/actions/Root.java
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,4 @@ public String toString() {
public void repr(SkylarkPrinter printer) {
printer.append(isSourceRoot() ? "<source root>" : "<derived root>");
}

@Override
public void reprLegacy(SkylarkPrinter printer) {
printer.append(toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -582,12 +582,6 @@ public boolean isImmutable() {
public void repr(SkylarkPrinter printer) {
printer.append("<rule collection for " + skylarkRuleContext.ruleLabelCanonicalName + ">");
}

@Override
public void reprLegacy(SkylarkPrinter printer) {
printer.append("rule_collection:");
printer.repr(skylarkRuleContext);
}
}

private void addOutput(HashMap<String, Object> outputsBuilder, String key, Object value)
Expand All @@ -611,10 +605,6 @@ public void repr(SkylarkPrinter printer) {
printer.append("<rule context for " + ruleLabelCanonicalName + ">");
}
}
@Override
public void reprLegacy(SkylarkPrinter printer) {
printer.append(ruleLabelCanonicalName);
}

/**
* Returns the original ruleContext.
Expand Down
10 changes: 0 additions & 10 deletions src/main/java/com/google/devtools/build/lib/cmdline/Label.java
Original file line number Diff line number Diff line change
Expand Up @@ -559,23 +559,13 @@ public boolean isImmutable() {
return true;
}

@Override
public void reprLegacy(SkylarkPrinter printer) {
printer.repr(getCanonicalForm());
}

@Override
public void repr(SkylarkPrinter printer) {
printer.append("Label(");
printer.repr(getCanonicalForm());
printer.append(")");
}

@Override
public void strLegacy(SkylarkPrinter printer) {
printer.append(getCanonicalForm());
}

@Override
public void str(SkylarkPrinter printer) {
printer.append(getCanonicalForm());
Expand Down
18 changes: 0 additions & 18 deletions src/main/java/com/google/devtools/build/lib/packages/Info.java
Original file line number Diff line number Diff line change
Expand Up @@ -176,24 +176,6 @@ public void repr(SkylarkPrinter printer) {
printer.append(")");
}

@Override
public void reprLegacy(SkylarkPrinter printer) {
boolean first = true;
printer.append(provider.getPrintableName());
printer.append("(");
// Sort by key to ensure deterministic output.
for (String key : Ordering.natural().sortedCopy(getKeys())) {
if (!first) {
printer.append(", ");
}
first = false;
printer.append(key);
printer.append(" = ");
printer.repr(getValue(key));
}
printer.append(")");
}

@Override
public String toString() {
return Printer.repr(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,6 @@ public void repr(SkylarkPrinter printer) {
printer.append("<aspect>");
}

@Override
public void reprLegacy(SkylarkPrinter printer) {
printer.append("Aspect:");
printer.repr(implementation);
}

public String getName() {
return getAspectClass().getName();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ public void serialize(SkylarkSemantics semantics, CodedOutputStream codedOut)
codedOut.writeBoolNoTag(semantics.incompatibleCheckedArithmetic());
codedOut.writeBoolNoTag(semantics.incompatibleComprehensionVariablesDoNotLeak());
codedOut.writeBoolNoTag(semantics.incompatibleDepsetIsNotIterable());
codedOut.writeBoolNoTag(semantics.incompatibleDescriptiveStringRepresentations());
codedOut.writeBoolNoTag(semantics.incompatibleDictLiteralHasNoDuplicates());
codedOut.writeBoolNoTag(semantics.incompatibleDisallowDictPlus());
codedOut.writeBoolNoTag(semantics.incompatibleDisallowKeywordOnlyArgs());
Expand All @@ -68,7 +67,6 @@ public SkylarkSemantics deserialize(CodedInputStream codedIn)
builder.incompatibleCheckedArithmetic(codedIn.readBool());
builder.incompatibleComprehensionVariablesDoNotLeak(codedIn.readBool());
builder.incompatibleDepsetIsNotIterable(codedIn.readBool());
builder.incompatibleDescriptiveStringRepresentations(codedIn.readBool());
builder.incompatibleDictLiteralHasNoDuplicates(codedIn.readBool());
builder.incompatibleDisallowDictPlus(codedIn.readBool());
builder.incompatibleDisallowKeywordOnlyArgs(codedIn.readBool());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,20 +112,6 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
)
public boolean incompatibleDepsetIsNotIterable;

@Option(
name = "incompatible_descriptive_string_representations",
defaultValue = "true",
category = "incompatible changes",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
help =
"If set to true, objects are converted to strings by `str` and `repr` functions using the "
+ "new style representations that are designed to be more descriptive and not to leak "
+ "information that's not supposed to be exposed."
)
public boolean incompatibleDescriptiveStringRepresentations;

@Option(
name = "incompatible_dict_literal_has_no_duplicates",
defaultValue = "true",
Expand Down Expand Up @@ -265,7 +251,6 @@ public SkylarkSemantics toSkylarkSemantics() {
.incompatibleCheckedArithmetic(incompatibleCheckedArithmetic)
.incompatibleComprehensionVariablesDoNotLeak(incompatibleComprehensionVariablesDoNotLeak)
.incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable)
.incompatibleDescriptiveStringRepresentations(incompatibleDescriptiveStringRepresentations)
.incompatibleDictLiteralHasNoDuplicates(incompatibleDictLiteralHasNoDuplicates)
.incompatibleDisallowDictPlus(incompatibleDisallowDictPlus)
.incompatibleDisallowKeywordOnlyArgs(incompatibleDisallowKeywordOnlyArgs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,6 @@ default boolean isImmutable() {
*/
void repr(SkylarkPrinter printer);

/**
* Print a legacy representation of object x.
*
* <p>By default dispatches to the {@code repr} method. Should be called instead of {@code repr}
* if --incompatible_descriptive_string_representations=false is used.
*
* @param printer an instance of a printer to be used for formatting nested values
*/
default void reprLegacy(SkylarkPrinter printer) {
repr(printer);
}

/**
* Print an informal, human-readable representation of the value.
*
Expand All @@ -62,15 +50,4 @@ default void reprLegacy(SkylarkPrinter printer) {
default void str(SkylarkPrinter printer) {
repr(printer);
}

/**
* Print a legacy informal, human-readable representation of the value.
*
* <p>By default dispatches to the {@code reprLegacy} method.
*
* @param printer a printer to be used for formatting nested values.
*/
default void strLegacy(SkylarkPrinter printer) {
reprLegacy(printer);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -569,9 +569,4 @@ public boolean isImmutable() {
public void repr(SkylarkPrinter printer) {
printer.append("<function " + getName() + ">");
}

@Override
public void reprLegacy(SkylarkPrinter printer) {
printer.append("<function " + getName() + ">");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -427,9 +427,9 @@ private static Object percent(Object lval, Object rval, Environment env, Locatio
String pattern = (String) lval;
try {
if (rval instanceof Tuple) {
return Printer.getPrinter(env).formatWithList(pattern, (Tuple) rval).toString();
return Printer.formatWithList(pattern, (Tuple) rval);
}
return Printer.getPrinter(env).format(pattern, rval).toString();
return Printer.format(pattern, rval);
} catch (IllegalFormatException e) {
throw new EvalException(location, e.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,9 @@ public final class FormatParser {
private static final ImmutableSet<Character> ILLEGAL_IN_FIELD =
ImmutableSet.of('.', '[', ']', ',');

private final Environment environment;
private final Location location;

public FormatParser(Environment environment, Location location) {
this.environment = environment;
public FormatParser(Location location) {
this.location = location;
}

Expand Down Expand Up @@ -93,7 +91,7 @@ protected int processOpeningBrace(
History history,
StringBuilder output)
throws EvalException {
BasePrinter printer = Printer.getPrinter(environment, output);
BasePrinter printer = Printer.getPrinter(output);
if (has(chars, pos + 1, '{')) {
// Escaped brace -> output and move to char after right brace
printer.append("{");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -951,8 +951,7 @@ public Boolean invoke(String self, String sub, Integer start, Object end)
defaultValue = "{}",
doc = "Dictionary of arguments."
),
useLocation = true,
useEnvironment = true
useLocation = true
)
private static final BuiltinFunction format =
new BuiltinFunction("format") {
Expand All @@ -961,10 +960,9 @@ public String invoke(
String self,
SkylarkList<Object> args,
SkylarkDict<?, ?> kwargs,
Location loc,
Environment env)
Location loc)
throws EvalException {
return new FormatParser(env, loc)
return new FormatParser(loc)
.format(
self,
args.getImmutableList(),
Expand Down Expand Up @@ -1640,13 +1638,12 @@ public Integer invoke(Object x, Location loc, Environment env) throws EvalExcept
"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.")},
useEnvironment = true
parameters = {@Param(name = "x", doc = "The object to convert.")}
)
private static final BuiltinFunction str =
new BuiltinFunction("str") {
public String invoke(Object x, Environment env) {
return Printer.getPrinter(env).str(x).toString();
public String invoke(Object x) {
return Printer.str(x);
}
};

Expand All @@ -1656,13 +1653,12 @@ public String invoke(Object x, Environment env) {
doc =
"Converts any object to a string representation. This is useful for debugging.<br>"
+ "<pre class=\"language-python\">repr(\"ab\") == '\"ab\"'</pre>",
parameters = {@Param(name = "x", doc = "The object to convert.")},
useEnvironment = true
parameters = {@Param(name = "x", doc = "The object to convert.")}
)
private static final BuiltinFunction repr =
new BuiltinFunction("repr") {
public String invoke(Object x, Environment env) {
return Printer.getPrinter(env).repr(x).toString();
public String invoke(Object x) {
return Printer.repr(x);
}
};

Expand Down Expand Up @@ -2150,7 +2146,7 @@ public Runtime.NoneType invoke(
String msg =
starargs
.stream()
.map((Object o) -> Printer.getPrinter(env).str(o).toString())
.map(Printer::str)
.collect(joining(sep));
// As part of the integration test "skylark_flag_test.sh", if the
// "--internal_skylark_flag_test_canary" flag is enabled, append an extra marker string to
Expand Down
60 changes: 0 additions & 60 deletions src/main/java/com/google/devtools/build/lib/syntax/Printer.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,35 +69,6 @@ public static BasePrinter getPrinter() {
return getPrinter(new StringBuilder());
}

/**
* Creates an instance of BasePrinter with a given buffer.
*
* @param env {@link Environment}
* @param buffer an {@link Appendable}
* @return new BasePrinter
*/
static BasePrinter getPrinter(Environment env, Appendable buffer) {
if (env.getSemantics().incompatibleDescriptiveStringRepresentations()) {
return new BasePrinter(buffer);
} else {
return new LegacyPrinter(buffer);
}
}

/**
* Creates an instance of BasePrinter with an empty buffer.
*
* @param env {@link Environment}
* @return new BasePrinter
*/
static BasePrinter getPrinter(Environment env) {
if (env.getSemantics().incompatibleDescriptiveStringRepresentations()) {
return new BasePrinter();
} else {
return new LegacyPrinter();
}
}

private Printer() {}

// These static methods proxy to the similar methods of BasePrinter
Expand Down Expand Up @@ -615,37 +586,6 @@ BasePrinter append(CharSequence sequence, int start, int end) {
}
}

/** A version of BasePrinter that renders object in old style for compatibility reasons. */
static final class LegacyPrinter extends BasePrinter {
protected LegacyPrinter() {
super();
}

protected LegacyPrinter(Appendable buffer) {
super(buffer);
}

@Override
public LegacyPrinter repr(Object o) {
if (o instanceof SkylarkValue) {
((SkylarkValue) o).reprLegacy(this);
} else {
super.repr(o);
}
return this;
}

@Override
public LegacyPrinter str(Object o) {
if (o instanceof SkylarkValue) {
((SkylarkValue) o).strLegacy(this);
} else {
super.str(o);
}
return this;
}
}

/** A version of {@code BasePrinter} that is able to print abbreviated lists. */
public static final class LengthLimitedPrinter extends BasePrinter {

Expand Down
Loading

0 comments on commit cd6d8ae

Please sign in to comment.