Allow dicts to contain non-comparable objects as keys
RELNOTES: Skylark dicts internally don't rely on keys order anymore and accept
any hashable values (i.e. structs with immutable values) as keys. Iteration order of dictionaries is no longer specified.
--
PiperOrigin-RevId: 141055080
MOS_MIGRATED_REVID=141055080
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index ef13e49..72ecf59 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -1059,9 +1059,7 @@
Collection<Target> targets = context.pkgBuilder.getTargets();
Location loc = ast.getLocation();
- // Sort by name.
- SkylarkDict<String, SkylarkDict<String, Object>> rules =
- SkylarkDict.<String, SkylarkDict<String, Object>>of(env);
+ SkylarkDict<String, SkylarkDict<String, Object>> rules = SkylarkDict.of(env);
for (Target t : targets) {
if (t instanceof Rule) {
SkylarkDict<String, Object> m = targetDict(t, loc, env);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index 9aa6b26..c14424a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -91,6 +91,8 @@
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.protobuf.TextFormat;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -102,20 +104,20 @@
public class SkylarkRuleClassFunctions {
@SkylarkSignature(
- name = "DATA_CFG",
- returnType = ConfigurationTransition.class,
- doc =
- "Deprecated. Use string \"data\" instead. "
- + "Specifies a transition to the data configuration."
+ name = "DATA_CFG",
+ returnType = ConfigurationTransition.class,
+ doc =
+ "Deprecated. Use string \"data\" instead. "
+ + "Specifies a transition to the data configuration."
)
private static final Object dataTransition = ConfigurationTransition.DATA;
@SkylarkSignature(
- name = "HOST_CFG",
- returnType = ConfigurationTransition.class,
- doc =
- "Deprecated. Use string \"host\" instead. "
- + "Specifies a transition to the host configuration."
+ name = "HOST_CFG",
+ returnType = ConfigurationTransition.class,
+ doc =
+ "Deprecated. Use string \"host\" instead. "
+ + "Specifies a transition to the host configuration."
)
private static final Object hostTransition = ConfigurationTransition.HOST;
@@ -125,15 +127,15 @@
// (except for transition phase) it's probably OK.
private static final LoadingCache<String, Label> labelCache =
CacheBuilder.newBuilder().build(new CacheLoader<String, Label>() {
- @Override
- public Label load(String from) throws Exception {
- try {
- return Label.parseAbsolute(from, false);
- } catch (LabelSyntaxException e) {
- throw new Exception(from);
- }
- }
- });
+ @Override
+ public Label load(String from) throws Exception {
+ try {
+ return Label.parseAbsolute(from, false);
+ } catch (LabelSyntaxException e) {
+ throw new Exception(from);
+ }
+ }
+ });
// TODO(bazel-team): Remove the code duplication (BaseRuleClasses and this class).
/** Parent rule class for non-executable non-test Skylark rules. */
@@ -241,114 +243,119 @@
// TODO(bazel-team): implement attribute copy and other rule properties
@SkylarkSignature(
- name = "rule",
- doc =
- "Creates a new rule. Store it in a global value, so that it can be loaded and called "
- + "from BUILD files.",
- returnType = BaseFunction.class,
- parameters = {
- @Param(
- name = "implementation",
- type = BaseFunction.class,
- doc =
- "the function implementing this rule, must have exactly one parameter: "
- + "<a href=\"ctx.html\">ctx</a>. The function is called during the analysis phase "
- + "for each instance of the rule. It can access the attributes provided by the "
- + "user. It must create actions to generate all the declared outputs."
- ),
- @Param(
- name = "test",
- type = Boolean.class,
- defaultValue = "False",
- doc =
- "Whether this rule is a test rule. "
- + "If True, the rule must end with <code>_test</code> (otherwise it must not), "
- + "and there must be an action that generates <code>ctx.outputs.executable</code>."
- ),
- @Param(
- name = "attrs",
- type = SkylarkDict.class,
- noneable = true,
- defaultValue = "None",
- doc =
- "dictionary to declare all the attributes of the rule. It maps from an attribute name "
- + "to an attribute object (see <a href=\"attr.html\">attr</a> module). "
- + "Attributes starting with <code>_</code> are private, and can be used to add "
- + "an implicit dependency on a label. The attribute <code>name</code> is "
- + "implicitly added and must not be specified. Attributes <code>visibility</code>, "
- + "<code>deprecation</code>, <code>tags</code>, <code>testonly</code>, and "
- + "<code>features</code> are implicitly added and might be overriden."
- ),
- // TODO(bazel-team): need to give the types of these builtin attributes
- @Param(
- name = "outputs",
- type = SkylarkDict.class,
- callbackEnabled = true,
- noneable = true,
- defaultValue = "None",
- doc =
- "outputs of this rule. "
- + "It is a dictionary mapping from string to a template name. "
- + "For example: <code>{\"ext\": \"%{name}.ext\"}</code>. <br>"
- + "The dictionary key becomes an attribute in <code>ctx.outputs</code>. "
- + "Similar to computed dependency rule attributes, you can also specify the name "
- + "of a function that returns the dictionary. This function can access all rule "
- + "attributes that are listed as parameters in its function signature. "
- + "For example, <code>outputs = _my_func<code> with <code>def _my_func(srcs, deps):"
- + "</code> has access to the attributes 'srcs' and 'deps' (if defined)."
- ),
- @Param(
- name = "executable",
- type = Boolean.class,
- defaultValue = "False",
- doc =
- "whether this rule is marked as executable or not. If True, "
- + "there must be an action that generates <code>ctx.outputs.executable</code>."
- ),
- @Param(
- name = "output_to_genfiles",
- type = Boolean.class,
- defaultValue = "False",
- doc =
- "If true, the files will be generated in the genfiles directory instead of the "
- + "bin directory. Unless you need it for compatibility with existing rules "
- + "(e.g. when generating header files for C++), do not set this flag."
- ),
- @Param(
- name = "fragments",
- type = SkylarkList.class,
- generic1 = String.class,
- defaultValue = "[]",
- doc =
- "List of names of configuration fragments that the rule requires "
- + "in target configuration."
- ),
- @Param(
- name = "host_fragments",
- type = SkylarkList.class,
- generic1 = String.class,
- defaultValue = "[]",
- doc =
- "List of names of configuration fragments that the rule requires "
- + "in host configuration."
- ),
- @Param(
- name = "_skylark_testable",
- type = Boolean.class,
- defaultValue = "False",
- doc =
- "<i>(Experimental)</i><br/><br/>"
- + "If true, this rule will expose its actions for inspection by rules that depend "
- + "on it via an <a href=\"globals.html#Actions\">Actions</a> provider. "
- + "The provider is also available to the rule itself by calling "
- + "<a href=\"ctx.html#created_actions\">ctx.created_actions()</a>."
- + "<br/><br/>"
- + "This should only be used for testing the analysis-time behavior of Skylark "
- + "rules. This flag may be removed in the future."
- )
- },
- useAst = true,
- useEnvironment = true
+ name = "rule",
+ doc =
+ "Creates a new rule. Store it in a global value, so that it can be loaded and called "
+ + "from BUILD files.",
+ returnType = BaseFunction.class,
+ parameters = {
+ @Param(
+ name = "implementation",
+ type = BaseFunction.class,
+ doc =
+ "the function implementing this rule, must have exactly one parameter: "
+ + "<a href=\"ctx.html\">ctx</a>. The function is called during the analysis "
+ + "phase for each instance of the rule. It can access the attributes "
+ + "provided by the user. It must create actions to generate all the declared "
+ + "outputs."
+ ),
+ @Param(
+ name = "test",
+ type = Boolean.class,
+ defaultValue = "False",
+ doc =
+ "Whether this rule is a test rule. "
+ + "If True, the rule must end with <code>_test</code> (otherwise it must "
+ + "not), and there must be an action that generates "
+ + "<code>ctx.outputs.executable</code>."
+ ),
+ @Param(
+ name = "attrs",
+ type = SkylarkDict.class,
+ noneable = true,
+ defaultValue = "None",
+ doc =
+ "dictionary to declare all the attributes of the rule. It maps from an attribute "
+ + "name to an attribute object (see <a href=\"attr.html\">attr</a> module). "
+ + "Attributes starting with <code>_</code> are private, and can be used to "
+ + "add an implicit dependency on a label. The attribute <code>name</code> is "
+ + "implicitly added and must not be specified. Attributes "
+ + "<code>visibility</code>, <code>deprecation</code>, <code>tags</code>, "
+ + "<code>testonly</code>, and <code>features</code> are implicitly added and "
+ + "might be overriden."
+ ),
+ // TODO(bazel-team): need to give the types of these builtin attributes
+ @Param(
+ name = "outputs",
+ type = SkylarkDict.class,
+ callbackEnabled = true,
+ noneable = true,
+ defaultValue = "None",
+ doc =
+ "outputs of this rule. "
+ + "It is a dictionary mapping from string to a template name. "
+ + "For example: <code>{\"ext\": \"%{name}.ext\"}</code>. <br>"
+ + "The dictionary key becomes an attribute in <code>ctx.outputs</code>. "
+ + "Similar to computed dependency rule attributes, you can also specify the "
+ + "name of a function that returns the dictionary. This function can access "
+ + "all rule attributes that are listed as parameters in its function "
+ + "signature. For example, <code>outputs = _my_func<code> with "
+ + "<code>def _my_func(srcs, deps):</code> has access to the attributes "
+ + "'srcs' and 'deps' (if defined)."
+ ),
+ @Param(
+ name = "executable",
+ type = Boolean.class,
+ defaultValue = "False",
+ doc =
+ "whether this rule is marked as executable or not. If True, "
+ + "there must be an action that generates "
+ + "<code>ctx.outputs.executable</code>."
+ ),
+ @Param(
+ name = "output_to_genfiles",
+ type = Boolean.class,
+ defaultValue = "False",
+ doc =
+ "If true, the files will be generated in the genfiles directory instead of the "
+ + "bin directory. Unless you need it for compatibility with existing rules "
+ + "(e.g. when generating header files for C++), do not set this flag."
+ ),
+ @Param(
+ name = "fragments",
+ type = SkylarkList.class,
+ generic1 = String.class,
+ defaultValue = "[]",
+ doc =
+ "List of names of configuration fragments that the rule requires "
+ + "in target configuration."
+ ),
+ @Param(
+ name = "host_fragments",
+ type = SkylarkList.class,
+ generic1 = String.class,
+ defaultValue = "[]",
+ doc =
+ "List of names of configuration fragments that the rule requires "
+ + "in host configuration."
+ ),
+ @Param(
+ name = "_skylark_testable",
+ type = Boolean.class,
+ defaultValue = "False",
+ doc =
+ "<i>(Experimental)</i><br/><br/>"
+ + "If true, this rule will expose its actions for inspection by rules that "
+ + "depend on it via an <a href=\"globals.html#Actions\">Actions</a> "
+ + "provider. The provider is also available to the rule itself by calling "
+ + "<a href=\"ctx.html#created_actions\">ctx.created_actions()</a>."
+ + "<br/><br/>"
+ + "This should only be used for testing the analysis-time behavior of "
+ + "Skylark rules. This flag may be removed in the future."
+ )
+ },
+ useAst = true,
+ useEnvironment = true
)
private static final BuiltinFunction rule =
new BuiltinFunction("rule") {
@@ -453,56 +460,57 @@
@SkylarkSignature(name = "aspect", doc =
- "Creates a new aspect. The result of this function must be stored in a global value. "
- + "Please see the <a href=\"../aspects.md\">introduction to Aspects</a> for more details.",
- returnType = SkylarkAspect.class,
- parameters = {
- @Param(name = "implementation", type = BaseFunction.class,
- doc = "the function implementing this aspect. Must have two parameters: "
- + "<a href=\"Target.html\">Target</a> (the target to which the aspect is applied) and "
- + "<a href=\"ctx.html\">ctx</a>. Attributes of the target are available via ctx.rule "
- + " field. The function is called during the analysis phase for each application of "
- + "an aspect to a target."
- ),
- @Param(name = "attr_aspects", type = SkylarkList.class, generic1 = String.class,
- defaultValue = "[]",
- doc = "List of attribute names. The aspect propagates along dependencies specified by "
- + " attributes of a target with this name. The list can also contain a single string '*':"
- + " in that case aspect propagates along all dependencies of a target."
- ),
- @Param(name = "attrs", type = SkylarkDict.class, noneable = true, defaultValue = "None",
- doc = "dictionary to declare all the attributes of the aspect. "
- + "It maps from an attribute name to an attribute object "
- + "(see <a href=\"attr.html\">attr</a> module). "
- + "Aspect attributes are available to implementation function as fields of ctx parameter. "
- + "Implicit attributes starting with <code>_</code> must have default values, and have "
- + "type <code>label</code> or <code>label_list</code>. "
- + "Explicit attributes must have type <code>string</code>, and must use the "
- + "<code>values</code> restriction. If explicit attributes are present, the aspect can "
- + "only be used with rules that have attributes of the same name and type, with valid "
- + "values. "
- ),
- @Param(
- name = "fragments",
- type = SkylarkList.class,
- generic1 = String.class,
- defaultValue = "[]",
- doc =
- "List of names of configuration fragments that the aspect requires "
- + "in target configuration."
- ),
- @Param(
- name = "host_fragments",
- type = SkylarkList.class,
- generic1 = String.class,
- defaultValue = "[]",
- doc =
- "List of names of configuration fragments that the aspect requires "
- + "in host configuration."
- )
- },
- useEnvironment = true,
- useAst = true
+ "Creates a new aspect. The result of this function must be stored in a global value. "
+ + "Please see the <a href=\"../aspects.md\">introduction to Aspects</a> for more "
+ + "details.",
+ returnType = SkylarkAspect.class,
+ parameters = {
+ @Param(name = "implementation", type = BaseFunction.class,
+ doc = "the function implementing this aspect. Must have two parameters: "
+ + "<a href=\"Target.html\">Target</a> (the target to which the aspect is "
+ + "applied) and <a href=\"ctx.html\">ctx</a>. Attributes of the target are "
+ + "available via ctx.rule field. The function is called during the analysis "
+ + "phase for each application of an aspect to a target."
+ ),
+ @Param(name = "attr_aspects", type = SkylarkList.class, generic1 = String.class,
+ defaultValue = "[]",
+ doc = "List of attribute names. The aspect propagates along dependencies specified "
+ + "by attributes of a target with this name. The list can also contain a single "
+ + "string '*': in that case aspect propagates along all dependencies of a target."
+ ),
+ @Param(name = "attrs", type = SkylarkDict.class, noneable = true, defaultValue = "None",
+ doc = "dictionary to declare all the attributes of the aspect. "
+ + "It maps from an attribute name to an attribute object "
+ + "(see <a href=\"attr.html\">attr</a> module). "
+ + "Aspect attributes are available to implementation function as fields of ctx "
+ + "parameter. Implicit attributes starting with <code>_</code> must have default "
+ + "values, and have type <code>label</code> or <code>label_list</code>. "
+ + "Explicit attributes must have type <code>string</code>, and must use the "
+ + "<code>values</code> restriction. If explicit attributes are present, the "
+ + "aspect can only be used with rules that have attributes of the same name and "
+ + "type, with valid values."
+ ),
+ @Param(
+ name = "fragments",
+ type = SkylarkList.class,
+ generic1 = String.class,
+ defaultValue = "[]",
+ doc =
+ "List of names of configuration fragments that the aspect requires "
+ + "in target configuration."
+ ),
+ @Param(
+ name = "host_fragments",
+ type = SkylarkList.class,
+ generic1 = String.class,
+ defaultValue = "[]",
+ doc =
+ "List of names of configuration fragments that the aspect requires "
+ + "in host configuration."
+ )
+ },
+ useEnvironment = true,
+ useAst = true
)
private static final BuiltinFunction aspect =
new BuiltinFunction("aspect") {
@@ -556,7 +564,7 @@
ast.getLocation(),
String.format(
"Aspect parameter attribute '%s' must have type 'string' and use the "
- + "'values' restriction.",
+ + "'values' restriction.",
nativeName));
}
if (!hasDefault) {
@@ -629,7 +637,7 @@
// TODO(dslomov): If a Skylark parameter extractor is specified for this aspect, its
// attributes may not be required.
for (Map.Entry<String, ImmutableSet<String>> attrRequirements :
- attribute.getRequiredAspectParameters().entrySet()) {
+ attribute.getRequiredAspectParameters().entrySet()) {
for (String required : attrRequirements.getValue()) {
if (!ruleClass.hasAttr(required, Type.STRING)) {
throw new EvalException(definitionLocation, String.format(
@@ -649,7 +657,7 @@
if (pkgContext == null) {
throw new EvalException(ast.getLocation(),
"Cannot instantiate a rule when loading a .bzl file. Rules can only be called from "
- + "a BUILD file (possibly via a macro).");
+ + "a BUILD file (possibly via a macro).");
}
return RuleFactory.createAndAddRule(
pkgContext,
@@ -738,39 +746,39 @@
objectType = Label.class,
parameters = {@Param(name = "label_string", type = String.class,
doc = "the label string"),
- @Param(
- name = "relative_to_caller_repository",
- type = Boolean.class,
- defaultValue = "False",
- named = true,
- positional = false,
- doc = "whether the label should be resolved relative to the label of the file this "
- + "function is called from.")},
+ @Param(
+ name = "relative_to_caller_repository",
+ type = Boolean.class,
+ defaultValue = "False",
+ named = true,
+ positional = false,
+ doc = "whether the label should be resolved relative to the label of the file this "
+ + "function is called from.")},
useLocation = true,
useEnvironment = true)
private static final BuiltinFunction label = new BuiltinFunction("Label") {
- @SuppressWarnings({"unchecked", "unused"})
- public Label invoke(
- String labelString, Boolean relativeToCallerRepository, Location loc, Environment env)
- throws EvalException {
- Label parentLabel = null;
- if (relativeToCallerRepository) {
- parentLabel = env.getCallerLabel();
- } else {
- parentLabel = env.getGlobals().label();
- }
- try {
- if (parentLabel != null) {
- LabelValidator.parseAbsoluteLabel(labelString);
- labelString = parentLabel.getRelative(labelString)
- .getUnambiguousCanonicalForm();
- }
- return labelCache.get(labelString);
- } catch (LabelValidator.BadLabelException | LabelSyntaxException | ExecutionException e) {
- throw new EvalException(loc, "Illegal absolute label syntax: " + labelString);
- }
+ @SuppressWarnings({"unchecked", "unused"})
+ public Label invoke(
+ String labelString, Boolean relativeToCallerRepository, Location loc, Environment env)
+ throws EvalException {
+ Label parentLabel = null;
+ if (relativeToCallerRepository) {
+ parentLabel = env.getCallerLabel();
+ } else {
+ parentLabel = env.getGlobals().label();
}
- };
+ try {
+ if (parentLabel != null) {
+ LabelValidator.parseAbsoluteLabel(labelString);
+ labelString = parentLabel.getRelative(labelString)
+ .getUnambiguousCanonicalForm();
+ }
+ return labelCache.get(labelString);
+ } catch (LabelValidator.BadLabelException | LabelSyntaxException | ExecutionException e) {
+ throw new EvalException(loc, "Illegal absolute label syntax: " + labelString);
+ }
+ }
+ };
// We want the Label ctor to show up under the Label documentation, but to be a "global
// function." Thus, we create a global Label object here, which just points to the Skylark
@@ -781,18 +789,18 @@
@SkylarkSignature(name = "FileType",
doc = "Deprecated. Creates a file filter from a list of strings. For example, to match "
- + "files ending with .cc or .cpp, use: "
- + "<pre class=language-python>FileType([\".cc\", \".cpp\"])</pre>",
+ + "files ending with .cc or .cpp, use: "
+ + "<pre class=language-python>FileType([\".cc\", \".cpp\"])</pre>",
returnType = SkylarkFileType.class,
objectType = SkylarkFileType.class,
parameters = {
- @Param(name = "types", type = SkylarkList.class, generic1 = String.class, defaultValue = "[]",
- doc = "a list of the accepted file extensions")})
+ @Param(name = "types", type = SkylarkList.class, generic1 = String.class,
+ defaultValue = "[]", doc = "a list of the accepted file extensions")})
private static final BuiltinFunction fileType = new BuiltinFunction("FileType") {
- public SkylarkFileType invoke(SkylarkList types) throws EvalException {
- return SkylarkFileType.of(types.getContents(String.class, "types"));
- }
- };
+ public SkylarkFileType invoke(SkylarkList types) throws EvalException {
+ return SkylarkFileType.of(types.getContents(String.class, "types"));
+ }
+ };
// We want the FileType ctor to show up under the FileType documentation, but to be a "global
// function." Thus, we create a global FileType object here, which just points to the Skylark
@@ -805,6 +813,7 @@
doc = "Creates a text message from the struct parameter. This method only works if all "
+ "struct elements (recursively) are strings, ints, booleans, other structs or a "
+ "list of these types. Quotes and new lines in strings are escaped. "
+ + "Keys are iterated in the sorted order. "
+ "Examples:<br><pre class=language-python>"
+ "struct(key=123).to_proto()\n# key: 123\n\n"
+ "struct(key=True).to_proto()\n# key: true\n\n"
@@ -818,59 +827,62 @@
+ "# key {\n# inner_key {\n# inner_inner_key: \"text\"\n# }\n# }\n</pre>",
objectType = SkylarkClassObject.class, returnType = String.class,
parameters = {
- // TODO(bazel-team): shouldn't we accept any ClassObject?
- @Param(name = "self", type = SkylarkClassObject.class,
- doc = "this struct")},
+ // TODO(bazel-team): shouldn't we accept any ClassObject?
+ @Param(name = "self", type = SkylarkClassObject.class,
+ doc = "this struct")},
useLocation = true)
private static final BuiltinFunction toProto = new BuiltinFunction("to_proto") {
- public String invoke(SkylarkClassObject self, Location loc) throws EvalException {
- StringBuilder sb = new StringBuilder();
- printProtoTextMessage(self, sb, 0, loc);
- return sb.toString();
- }
+ public String invoke(SkylarkClassObject self, Location loc) throws EvalException {
+ StringBuilder sb = new StringBuilder();
+ printProtoTextMessage(self, sb, 0, loc);
+ return sb.toString();
+ }
- private void printProtoTextMessage(ClassObject object, StringBuilder sb,
- int indent, Location loc) throws EvalException {
- for (String key : object.getKeys()) {
- printProtoTextMessage(key, object.getValue(key), sb, indent, loc);
- }
+ private void printProtoTextMessage(
+ ClassObject object, StringBuilder sb, int indent, Location loc) throws EvalException {
+ // For determinism sort the keys alphabetically
+ List<String> keys = new ArrayList<>(object.getKeys());
+ Collections.sort(keys);
+ for (String key : keys) {
+ printProtoTextMessage(key, object.getValue(key), sb, indent, loc);
}
+ }
- private void printProtoTextMessage(String key, Object value, StringBuilder sb,
- int indent, Location loc, String container) throws EvalException {
- if (value instanceof ClassObject) {
- print(sb, key + " {", indent);
- printProtoTextMessage((ClassObject) value, sb, indent + 1, loc);
- print(sb, "}", indent);
- } else if (value instanceof String) {
- print(sb,
- key + ": \"" + escapeDoubleQuotesAndBackslashesAndNewlines((String) value) + "\"",
- indent);
- } else if (value instanceof Integer) {
- print(sb, key + ": " + value, indent);
- } else if (value instanceof Boolean) {
- // We're relying on the fact that Java converts Booleans to Strings in the same way
- // as the protocol buffers do.
- print(sb, key + ": " + value, indent);
- } else {
- throw new EvalException(loc,
- "Invalid text format, expected a struct, a string, a bool, or an int but got a "
- + EvalUtils.getDataTypeName(value) + " for " + container + " '" + key + "'");
- }
+ private void printProtoTextMessage(String key, Object value, StringBuilder sb,
+ int indent, Location loc, String container) throws EvalException {
+ if (value instanceof ClassObject) {
+ print(sb, key + " {", indent);
+ printProtoTextMessage((ClassObject) value, sb, indent + 1, loc);
+ print(sb, "}", indent);
+ } else if (value instanceof String) {
+ print(sb,
+ key + ": \"" + escapeDoubleQuotesAndBackslashesAndNewlines((String) value) + "\"",
+ indent);
+ } else if (value instanceof Integer) {
+ print(sb, key + ": " + value, indent);
+ } else if (value instanceof Boolean) {
+ // We're relying on the fact that Java converts Booleans to Strings in the same way
+ // as the protocol buffers do.
+ print(sb, key + ": " + value, indent);
+ } else {
+ throw new EvalException(loc,
+ "Invalid text format, expected a struct, a string, a bool, or an int but got a "
+ + EvalUtils.getDataTypeName(value) + " for " + container + " '" + key + "'");
}
+ }
- private void printProtoTextMessage(String key, Object value, StringBuilder sb,
- int indent, Location loc) throws EvalException {
- if (value instanceof SkylarkList) {
- for (Object item : ((SkylarkList) value)) {
- // TODO(bazel-team): There should be some constraint on the fields of the structs
- // in the same list but we ignore that for now.
- printProtoTextMessage(key, item, sb, indent, loc, "list element in struct field");
- }
- } else {
- printProtoTextMessage(key, value, sb, indent, loc, "struct field");
+ private void printProtoTextMessage(String key, Object value, StringBuilder sb,
+ int indent, Location loc) throws EvalException {
+ if (value instanceof SkylarkList) {
+ for (Object item : ((SkylarkList) value)) {
+ // TODO(bazel-team): There should be some constraint on the fields of the structs
+ // in the same list but we ignore that for now.
+ printProtoTextMessage(key, item, sb, indent, loc, "list element in struct field");
}
+ } else {
+ printProtoTextMessage(key, value, sb, indent, loc, "struct field");
}
+ }
private void print(StringBuilder sb, String text, int indent) {
for (int i = 0; i < indent; i++) {
@@ -878,8 +890,8 @@
}
sb.append(text);
sb.append("\n");
- }
- };
+ }
+ };
/**
* Escapes the given string for use in proto/JSON string.
@@ -980,13 +992,13 @@
}
)
private static final BuiltinFunction output_group = new BuiltinFunction("output_group") {
- public SkylarkNestedSet invoke(TransitiveInfoCollection self, String group) {
- OutputGroupProvider provider = self.getProvider(OutputGroupProvider.class);
- NestedSet<Artifact> result = provider != null
- ? provider.getOutputGroup(group)
- : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
- return SkylarkNestedSet.of(Artifact.class, result);
- }
+ public SkylarkNestedSet invoke(TransitiveInfoCollection self, String group) {
+ OutputGroupProvider provider = self.getProvider(OutputGroupProvider.class);
+ NestedSet<Artifact> result = provider != null
+ ? provider.getOutputGroup(group)
+ : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
+ return SkylarkNestedSet.of(Artifact.class, result);
+ }
};
static {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index c83ed98..c95df57 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -72,6 +73,9 @@
o1 = SkylarkType.convertToSkylark(o1, /*env=*/ null);
o2 = SkylarkType.convertToSkylark(o2, /*env=*/ null);
+ if (o1 instanceof ClassObject && o2 instanceof ClassObject) {
+ throw new ComparisonException("Cannot compare structs");
+ }
if (o1 instanceof SkylarkNestedSet && o2 instanceof SkylarkNestedSet) {
throw new ComparisonException("Cannot compare sets");
}
@@ -312,6 +316,15 @@
return ((SkylarkList) o).getImmutableList();
} else if (o instanceof Map) {
// For dictionaries we iterate through the keys only
+ if (o instanceof SkylarkDict) {
+ // SkylarkDicts handle ordering themselves
+ SkylarkDict<?, ?> dict = (SkylarkDict) o;
+ List<Object> list = Lists.newArrayListWithCapacity(dict.size());
+ for (Map.Entry<?, ?> entries : dict.entrySet()) {
+ list.add(entries.getKey());
+ }
+ return ImmutableList.copyOf(list);
+ }
// For determinism, we sort the keys.
try {
return SKYLARK_COMPARATOR.sortedCopy(((Map<?, ?>) o).keySet());
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 5748f63..7ca5793 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -801,7 +801,7 @@
ImmutableList.Builder<Object> posargs = new ImmutableList.Builder<>();
// We copy this into an ImmutableMap in the end, but we can't use an ImmutableMap.Builder, or
// we'd still have to have a HashMap on the side for the sake of properly handling duplicates.
- Map<String, Object> kwargs = new HashMap<>();
+ Map<String, Object> kwargs = new LinkedHashMap<>();
BaseFunction function = checkCallable(funcValue, getLocation());
evalArguments(posargs, kwargs, env);
return function.call(posargs.build(), ImmutableMap.copyOf(kwargs), this, env);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index b2a33c5..fd79759 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -1314,12 +1314,7 @@
+ "<code>popitem()</code> is useful to destructively iterate over a dictionary, "
+ "as often used in set algorithms. "
+ "If the dictionary is empty, calling <code>popitem()</code> fails. "
- + "Note that in Skylark, as opposed to Python, "
- + "the dictionary keys are actually sorted, "
- + "and it is deterministic which pair will returned: that with the first key, "
- + "according to the builtin total order. "
- + "Thus if keys are numbers, the smallest key is returned first; "
- + "if they are lists or strings, they are compared lexicographically, etc.",
+ + "It is deterministic which pair is returned.",
parameters = {
@Param(name = "self", type = SkylarkDict.class, doc = "This dict.")
},
@@ -1432,9 +1427,9 @@
objectType = SkylarkDict.class,
returnType = MutableList.class,
doc =
- "Returns the list of values. Dictionaries are always sorted by their keys:"
+ "Returns the list of values:"
+ "<pre class=\"language-python\">"
- + "{2: \"a\", 4: \"b\", 1: \"c\"}.values() == [\"c\", \"a\", \"b\"]</pre>\n",
+ + "{2: \"a\", 4: \"b\", 1: \"c\"}.values() == [\"a\", \"b\", \"c\"]</pre>\n",
parameters = {@Param(name = "self", type = SkylarkDict.class, doc = "This dict.")},
useEnvironment = true
)
@@ -1450,9 +1445,9 @@
objectType = SkylarkDict.class,
returnType = MutableList.class,
doc =
- "Returns the list of key-value tuples. Dictionaries are always sorted by their keys:"
+ "Returns the list of key-value tuples:"
+ "<pre class=\"language-python\">"
- + "{2: \"a\", 4: \"b\", 1: \"c\"}.items() == [(1, \"c\"), (2, \"a\"), (4, \"b\")]"
+ + "{2: \"a\", 4: \"b\", 1: \"c\"}.items() == [(2, \"a\"), (4, \"b\"), (1, \"c\")]"
+ "</pre>\n",
parameters = {@Param(name = "self", type = SkylarkDict.class, doc = "This dict.")},
useEnvironment = true
@@ -1470,8 +1465,8 @@
@SkylarkSignature(name = "keys", objectType = SkylarkDict.class,
returnType = MutableList.class,
- doc = "Returns the list of keys. Dictionaries are always sorted by their keys:"
- + "<pre class=\"language-python\">{2: \"a\", 4: \"b\", 1: \"c\"}.keys() == [1, 2, 4]"
+ doc = "Returns the list of keys:"
+ + "<pre class=\"language-python\">{2: \"a\", 4: \"b\", 1: \"c\"}.keys() == [2, 4, 1]"
+ "</pre>\n",
parameters = {
@Param(name = "self", type = SkylarkDict.class, doc = "This dict.")},
@@ -1482,9 +1477,11 @@
@SuppressWarnings("unchecked")
public MutableList<?> invoke(SkylarkDict<?, ?> self,
Environment env) throws EvalException {
- return new MutableList(
- Ordering.natural().sortedCopy((Set<Comparable<?>>) (Set<?>) self.keySet()),
- env);
+ List<Object> list = Lists.newArrayListWithCapacity(self.size());
+ for (Map.Entry<?, ?> entries : self.entrySet()) {
+ list.add(entries.getKey());
+ }
+ return new MutableList(list, env);
}
};
@@ -1678,8 +1675,7 @@
doc =
"Creates a <a href=\"#modules.dict\">dictionary</a> from an optional positional "
+ "argument and an optional set of keyword arguments. Values from the keyword argument "
- + "will overwrite values from the positional argument if a key appears multiple times. "
- + "Dictionaries are always sorted by their keys",
+ + "will overwrite values from the positional argument if a key appears multiple times.",
parameters = {
@Param(
name = "args",
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java
index 6ae2ae5..af75d43 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java
@@ -18,8 +18,8 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
import com.google.devtools.build.lib.syntax.SkylarkMutable.MutableMap;
+import java.util.LinkedHashMap;
import java.util.Map;
-import java.util.TreeMap;
import javax.annotation.Nullable;
/**
@@ -38,7 +38,7 @@
+ "d = {\"a\" : 1} + {\"b\" : 2} # d == {\"a\" : 1, \"b\" : 2}\n"
+ "d += {\"c\" : 3} # d == {\"a\" : 1, \"b\" : 2, \"c\" : 3}\n"
+ "d = d + {\"c\" : 5} # d == {\"a\" : 1, \"b\" : 2, \"c\" : 5}</pre>"
- + "Iterating on a dict is equivalent to iterating on its keys (in sorted order).<br>"
+ + "Iterating on a dict is equivalent to iterating on its keys (order is not specified).<br>"
+ "Dicts support the <code>in</code> operator, testing membership in the keyset of the dict. "
+ "Example:<br>"
+ "<pre class=\"language-python\">\"a\" in {\"a\" : 2, \"b\" : 5} # evaluates as True"
@@ -46,7 +46,7 @@
public final class SkylarkDict<K, V>
extends MutableMap<K, V> implements Map<K, V>, SkylarkIndexable {
- private final TreeMap<K, V> contents = new TreeMap<>(EvalUtils.SKYLARK_COMPARATOR);
+ private final LinkedHashMap<K, V> contents = new LinkedHashMap<>();
private final Mutability mutability;
@@ -139,7 +139,7 @@
/** @return the first key in the dict */
K firstKey() {
- return contents.firstKey();
+ return contents.entrySet().iterator().next().getKey();
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 17f5484..066c544 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -657,6 +657,11 @@
}
@Test
+ public void testProtoFieldsOrder() throws Exception {
+ checkTextMessage("struct(d=4, b=2, c=3, a=1).to_proto()", "a: 1", "b: 2", "c: 3", "d: 4");
+ }
+
+ @Test
public void testTextMessageEscapes() throws Exception {
checkTextMessage("struct(name='a\"b').to_proto()", "name: \"a\\\"b\"");
checkTextMessage("struct(name='a\\'b').to_proto()", "name: \"a'b\"");
@@ -810,6 +815,14 @@
}
@Test
+ public void testStructIncomparability() throws Exception {
+ checkErrorContains("Cannot compare structs", "struct(a = 1) < struct(a = 2)");
+ checkErrorContains("Cannot compare structs", "struct(a = 1) > struct(a = 2)");
+ checkErrorContains("Cannot compare structs", "struct(a = 1) <= struct(a = 2)");
+ checkErrorContains("Cannot compare structs", "struct(a = 1) >= struct(a = 2)");
+ }
+
+ @Test
public void testStructAccessingFieldsFromSkylark() throws Exception {
eval("x = struct(a = 1, b = 2)", "x1 = x.a", "x2 = x.b");
assertThat(lookup("x1")).isEqualTo(1);
@@ -934,6 +947,18 @@
}
@Test
+ public void testStructsInDicts() throws Exception {
+ eval("d = {struct(a = 1): 'aa', struct(b = 2): 'bb'}");
+ assertThat(eval("d[struct(a = 1)]")).isEqualTo("aa");
+ assertThat(eval("d[struct(b = 2)]")).isEqualTo("bb");
+ assertThat(eval("str([d[k] for k in d])")).isEqualTo("[\"aa\", \"bb\"]");
+
+ checkErrorContains(
+ "unhashable type: 'struct'",
+ "{struct(a = []): 'foo'}");
+ }
+
+ @Test
public void testStructMembersAreImmutable() throws Exception {
checkErrorContains(
"cannot assign to 's.x'",
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index b60dace..a36fc74 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -547,27 +547,27 @@
assertEquals(
SkylarkList.createImmutable(
ImmutableList.<String>of(
- "cmd",
- "compatible_with",
- "executable",
- "features",
+ "name",
+ "visibility",
+ "tags",
+ "generator_name",
"generator_function",
"generator_location",
- "generator_name",
- "heuristic_label_expansion",
- "kind",
- "local",
- "message",
- "name",
- "output_to_bindir",
- "outs",
+ "features",
+ "compatible_with",
"restricted_to",
"srcs",
- "stamp",
- "tags",
- "toolchains",
"tools",
- "visibility")),
+ "toolchains",
+ "outs",
+ "cmd",
+ "output_to_bindir",
+ "local",
+ "message",
+ "executable",
+ "stamp",
+ "heuristic_label_expansion",
+ "kind")),
result);
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index fa72014..9bdaa15 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -146,7 +146,11 @@
.testStatement("'hello' == 'bye'", false)
.testStatement("None == None", true)
.testStatement("[1, 2] == [1, 2]", true)
- .testStatement("[1, 2] == [2, 1]", false);
+ .testStatement("[1, 2] == [2, 1]", false)
+ .testStatement("{'a': 1, 'b': 2} == {'b': 2, 'a': 1}", true)
+ .testStatement("{'a': 1, 'b': 2} == {'a': 1}", false)
+ .testStatement("{'a': 1, 'b': 2} == {'a': 1, 'b': 2, 'c': 3}", false)
+ .testStatement("{'a': 1, 'b': 2} == {'a': 1, 'b': 3}", false);
}
@Test
@@ -157,7 +161,11 @@
.testStatement("'hello' != 'hel' + 'lo'", false)
.testStatement("'hello' != 'bye'", true)
.testStatement("[1, 2] != [1, 2]", false)
- .testStatement("[1, 2] != [2, 1]", true);
+ .testStatement("[1, 2] != [2, 1]", true)
+ .testStatement("{'a': 1, 'b': 2} != {'b': 2, 'a': 1}", false)
+ .testStatement("{'a': 1, 'b': 2} != {'a': 1}", true)
+ .testStatement("{'a': 1, 'b': 2} != {'a': 1, 'b': 2, 'c': 3}", true)
+ .testStatement("{'a': 1, 'b': 2} != {'a': 1, 'b': 3}", true);
}
@Test
@@ -272,7 +280,10 @@
.update(kwargs.getName(), kwargs)
.testEval(
"kwargs(foo=1, bar='bar', wiz=[1,2,3]).items()",
- "[('bar', 'bar'), ('foo', 1), ('wiz', [1, 2, 3])]");
+ "[('foo', 1), ('bar', 'bar'), ('wiz', [1, 2, 3])]")
+ .testEval(
+ "kwargs(wiz=[1,2,3], bar='bar', foo=1).items()",
+ "[('wiz', [1, 2, 3]), ('bar', 'bar'), ('foo', 1)]");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
index ea81adb..a001f3f 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -1311,8 +1311,8 @@
new BothModesTest()
.testEval("{1: 'foo'}.values()", "['foo']")
.testEval("{}.values()", "[]")
- .testEval("{True: 3, False: 5}.values()", "[5, 3]")
- .testEval("{'a': 5, 'c': 2, 'b': 4, 'd': 3}.values()", "[5, 4, 2, 3]");
+ .testEval("{True: 3, False: 5}.values()", "[3, 5]")
+ .testEval("{'a': 5, 'c': 2, 'b': 4, 'd': 3}.values()", "[5, 2, 4, 3]");
// sorted by keys
}
@@ -1321,9 +1321,9 @@
new BothModesTest()
.testEval("{1: 'foo'}.keys()", "[1]")
.testEval("{}.keys()", "[]")
- .testEval("{True: 3, False: 5}.keys()", "[False, True]")
+ .testEval("{True: 3, False: 5}.keys()", "[True, False]")
.testEval(
- "{1:'a', 2:'b', 6:'c', 0:'d', 5:'e', 4:'f', 3:'g'}.keys()", "[0, 1, 2, 3, 4, 5, 6]");
+ "{1:'a', 2:'b', 6:'c', 0:'d', 5:'e', 4:'f', 3:'g'}.keys()", "[1, 2, 6, 0, 5, 4, 3]");
}
@Test
@@ -1342,7 +1342,7 @@
.testEval("{'a': 'foo'}.items()", "[('a', 'foo')]")
.testEval("{}.items()", "[]")
.testEval("{1: 3, 2: 5}.items()", "[(1, 3), (2, 5)]")
- .testEval("{'a': 5, 'c': 2, 'b': 4}.items()", "[('a', 5), ('b', 4), ('c', 2)]");
+ .testEval("{'a': 5, 'c': 2, 'b': 4}.items()", "[('a', 5), ('c', 2), ('b', 4)]");
}
@Test
@@ -1378,9 +1378,9 @@
"popitem(): dictionary is empty",
"d = {2: 'bar', 3: 'baz', 1: 'foo'}\n"
+ "if len(d) != 3: fail('popitem 0')\n"
- + "if d.popitem() != (1, 'foo'): fail('popitem 1')\n"
+ "if d.popitem() != (2, 'bar'): fail('popitem 2')\n"
+ "if d.popitem() != (3, 'baz'): fail('popitem 3')\n"
+ + "if d.popitem() != (1, 'foo'): fail('popitem 1')\n"
+ "if d != {}: fail('popitem 4')\n"
+ "d.popitem()");
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index dcb927f..e949290 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -919,7 +919,7 @@
" for a in d:",
" s += a",
" return s",
- "s = foo()").testLookup("s", "abc");
+ "s = foo()").testLookup("s", "cab");
}
@Test
