commit ea6c82d7e47d9da9418d87b4e8228ce39dc4a726
author Florian Weikert <fwe@google.com> Mon Sep 05 12:15:31 2016 +0000
committer Kristina Chodorow <kchodorow@google.com> Tue Sep 06 15:38:43 2016 +0000
tree 23b0c4f5798584a670a5f692c207d06d8d00ca76
parent a34c4febbbdfba6c045598101ca9a491cfde7dd9

Roll forward of commit 19db71413329da3f5d22b5fc7681471f3d971d88 (Skylark: Replaced late bound attributes with computed default attributes) with two bug fixes:

1. Unlike SkylarkComputedDefault, SkylarkComputedDefaultTemplate did not sort the names of its attribute dependencies. Consequently, lookup operations failed when callback functions in bzl files specified the names of their required attributes in a non-alphabetical order since the order of the key tuples was different (e.g. [1, 2] vs [2, 1]).

It would be less error prone to always sort the dependencies in createDependencyAssignmentTuple(), but this would impact performance.

2. SkylarkCallbackFunction ignores the legacy "cfg" parameter in callback functions. This special case should be deleted once all cfg parameters have been removed from the depot.

--
MOS_MIGRATED_REVID=132235927

src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java

diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
index 3e2665d..ea52e11 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
@@ -22,7 +22,7 @@
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
-import com.google.devtools.build.lib.packages.Attribute.SkylarkLateBound;
+import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.SkylarkAspect;
 import com.google.devtools.build.lib.skylarkinterface.Param;
@@ -46,11 +46,9 @@
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.Preconditions;
-
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-
 import javax.annotation.Nullable;
 
 /**
@@ -163,10 +161,16 @@
     Object defaultValue = arguments.get(DEFAULT_ARG);
     if (!EvalUtils.isNullOrNone(defaultValue)) {
       if (defaultValue instanceof UserDefinedFunction) {
-        // Late bound attribute. Non label type attributes already caused a type check error.
+        // Computed attribute. Non label type attributes already caused a type check error.
+        SkylarkCallbackFunction callback =
+            new SkylarkCallbackFunction((UserDefinedFunction) defaultValue, ast, env);
+        // SkylarkComputedDefaultTemplate needs to know the names of all attributes that it depends
+        // on. However, this method does not know anything about other attributes.
+        // We solve this problem by asking the SkylarkCallbackFunction for the parameter names used
+        // in the function definition, which must be the names of attributes used by the callback.
         builder.value(
-            new SkylarkLateBound(
-                new SkylarkCallbackFunction((UserDefinedFunction) defaultValue, ast, env)));
+            new SkylarkComputedDefaultTemplate(
+                type, callback.getParameterNames(), callback, ast.getLocation()));
       } else {
         builder.defaultValue(defaultValue);
       }