Refactor Skylark Environment-s

Make Environment-s freezable: Introduce a class Mutability
as a revokable capability to mutate objects in an Environment.
For now, only Environment-s carry this capability.
Make sure that every Mutability is revoked in the same function that create...

This reinstates a change that previously rolled-back because it broke the
serializability of SkylarkLookupValue. Bad news: serializing it succeeds for the
wrong reason, because a SkylarkEnvironment was stored as a result (now an
Environment.Extension) that was Serializable but inherited its bindings from an Environment (now an Environment.BaseExtension) which wasn't Serializable.
Apparently, Java doesn't try to serialize the bindings then (or at least doesn't
error out when it fails), because these bindings map variable names to pretty
arbitrary objects, and a lot of those we find in practice aren't Serializable.
Thus the current code passes the same tests as the previous code, but obviously
the serialization is just as ineffective as it used to be.

--
MOS_MIGRATED_REVID=102776694

1 files changed, 51 insertions, 45 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
index 487570864d..8c073d2315 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
@@ -15,20 +15,24 @@
 package com.google.devtools.build.lib.packages;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
 import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
+import com.google.devtools.build.lib.syntax.BaseFunction;
+import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.FuncallExpression;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.syntax.Label.SyntaxException;
-import com.google.devtools.build.lib.syntax.StackTraceElement;
+import com.google.devtools.build.lib.syntax.UserDefinedFunction;
+import com.google.devtools.build.lib.util.Pair;
 
 import java.util.Map;
 import java.util.Set;
 
+import javax.annotation.Nullable;
+
 /**
  * Given a rule class and a set of attributes, returns a Rule instance. Also
  * performs a number of checks and associates the rule and the owning package
@@ -78,7 +82,7 @@ public class RuleFactory {
       EventHandler eventHandler,
       FuncallExpression ast,
       Location location,
-      ImmutableList<StackTraceElement> stackTrace)
+      @Nullable Environment env)
       throws InvalidRuleException {
     Preconditions.checkNotNull(ruleClass);
     String ruleClassName = ruleClass.getName();
@@ -108,31 +112,19 @@ public class RuleFactory {
     }
 
     try {
-      Rule rule = ruleClass.createRuleWithLabel(pkgBuilder, label,
-          addGeneratorAttributesForMacros(attributeValues, stackTrace), eventHandler, ast,
+      return ruleClass.createRuleWithLabel(
+          pkgBuilder,
+          label,
+          addGeneratorAttributesForMacros(attributeValues, env),
+          eventHandler,
+          ast,
           location);
-      return rule;
     } catch (SyntaxException e) {
       throw new RuleFactory.InvalidRuleException(ruleClass + " " + e.getMessage());
     }
   }
 
   /**
-   * Creates and returns a rule instance (without a stack trace).
-   */
-  static Rule createRule(
-      Package.Builder pkgBuilder,
-      RuleClass ruleClass,
-      Map<String, Object> attributeValues,
-      EventHandler eventHandler,
-      FuncallExpression ast,
-      Location location)
-      throws InvalidRuleException {
-    return createRule(pkgBuilder, ruleClass, attributeValues, eventHandler, ast, location,
-        ImmutableList.<StackTraceElement>of());
-  }
-
-  /**
    * Creates a rule instance, adds it to the package and returns it.
    *
    * @param pkgBuilder the under-construction package to which the rule belongs
@@ -145,34 +137,40 @@ public class RuleFactory {
    *        rule creation
    * @param ast the abstract syntax tree of the rule expression (optional)
    * @param location the location at which this rule was declared
-   * @param stackTrace the stack trace containing all functions that led to the creation of
-   *        this rule (optional)
    * @throws InvalidRuleException if the rule could not be constructed for any
    *         reason (e.g. no <code>name</code> attribute is defined)
-   * @throws NameConflictException
+   * @throws InvalidRuleException, NameConflictException
    */
-  static Rule createAndAddRule(Package.Builder pkgBuilder,
-                  RuleClass ruleClass,
-                  Map<String, Object> attributeValues,
-                  EventHandler eventHandler,
-                  FuncallExpression ast,
-                  Location location,
-                  ImmutableList<StackTraceElement> stackTrace)
+  static Rule createAndAddRule(
+      Package.Builder pkgBuilder,
+      RuleClass ruleClass,
+      Map<String, Object> attributeValues,
+      EventHandler eventHandler,
+      FuncallExpression ast,
+      Location location,
+      Environment env)
       throws InvalidRuleException, NameConflictException {
     Rule rule = createRule(
-        pkgBuilder, ruleClass, attributeValues, eventHandler, ast, location, stackTrace);
+        pkgBuilder, ruleClass, attributeValues, eventHandler, ast, location, env);
     pkgBuilder.addRule(rule);
     return rule;
   }
 
-  public static Rule createAndAddRule(PackageContext context,
+  public static Rule createAndAddRule(
+      PackageContext context,
       RuleClass ruleClass,
       Map<String, Object> attributeValues,
       FuncallExpression ast,
-      ImmutableList<StackTraceElement> stackTrace)
+      Environment env)
       throws InvalidRuleException, NameConflictException {
-    return createAndAddRule(context.pkgBuilder, ruleClass, attributeValues, context.eventHandler,
-        ast, ast.getLocation(), stackTrace);
+    return createAndAddRule(
+        context.pkgBuilder,
+        ruleClass,
+        attributeValues,
+        context.eventHandler,
+        ast,
+        ast.getLocation(),
+        env);
   }
 
   /**
@@ -192,22 +190,30 @@ public class RuleFactory {
    * <p>Otherwise, it returns the given attributes without any changes.
    */
   private static Map<String, Object> addGeneratorAttributesForMacros(
-      Map<String, Object> args, ImmutableList<StackTraceElement> stackTrace) {
-    if (stackTrace.size() <= 2 || args.containsKey("generator_name")
-        || args.containsKey("generator_function")) {
-      // Returns the original arguments if a) there is only the rule itself on the stack
-      // trace (=> no macro) or b) the attributes have already been set by Python pre-processing.
-      // The stack trace will always have at least two entries: one for the call to the rule and one
-      // for its implementation. Consequently, we check for size <= 2.
+      Map<String, Object> args, @Nullable Environment env) {
+    // Returns the original arguments if a) there is only the rule itself on the stack
+    // trace (=> no macro) or b) the attributes have already been set by Python pre-processing.
+    if (env == null) {
+      return args;
+    }
+    boolean hasName = args.containsKey("generator_name");
+    boolean hasFunc = args.containsKey("generator_function");
+    // TODO(bazel-team): resolve cases in our code where hasName && !hasFunc, or hasFunc && !hasName
+    if (hasName || hasFunc) {
+      return args;
+    }
+    Pair<FuncallExpression, BaseFunction> topCall = env.getTopCall();
+    if (topCall == null || !(topCall.second instanceof UserDefinedFunction)) {
       return args;
     }
 
-    StackTraceElement generator = stackTrace.get(0);
+    FuncallExpression generator = topCall.first;
+    BaseFunction function = topCall.second;
     String name = generator.getNameArg();
     ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
     builder.putAll(args);
     builder.put("generator_name", (name == null) ? args.get("name") : name);
-    builder.put("generator_function", generator.getName());
+    builder.put("generator_function", function.getName());
     builder.put("generator_location", Location.printPathAndLine(generator.getLocation()));
 
     try {