When a Skylark macro creates a native rule, it also sets the following rule attributes: generator_{function, name, location}

--
MOS_MIGRATED_REVID=102139196

12 files changed, 335 insertions, 109 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/events/Location.java b/src/main/java/com/google/devtools/build/lib/events/Location.java
index 608d307c68..74b04a913a 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Location.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Location.java
@@ -291,4 +291,30 @@ public abstract class Location implements Serializable {
       return null;
     }
   };
+
+  /**
+   * Returns the location in the format "filename:line".
+   *
+   * <p>If such a location is not defined, this method returns an empty string.
+   */
+  public static String printPathAndLine(Location location) {
+    return (location == null) ? "" : location.printPathAndLine();
+  }
+
+  /**
+   * Returns this location in the format "filename:line".
+   */
+  public String printPathAndLine() {
+    StringBuilder builder = new StringBuilder();
+    PathFragment path = getPath();
+    if (path != null) {
+      builder.append(path.getPathString());
+    }
+
+    LineAndColumn position = getStartLineAndColumn();
+    if (position != null) {
+      builder.append(":").append(position.getLine());
+    }
+    return builder.toString();
+  }
 }
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 b94147dc54..2122d42a8b 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
@@ -906,10 +906,11 @@ public final class PackageFactory {
                               String ruleClassName,
                               PackageContext context,
                               Map<String, Object> kwargs,
-                              FuncallExpression ast)
+                              FuncallExpression ast,
+                              Environment env)
       throws RuleFactory.InvalidRuleException, Package.NameConflictException {
     RuleClass ruleClass = getBuiltInRuleClass(ruleClassName, ruleFactory);
-    RuleFactory.createAndAddRule(context, ruleClass, kwargs, ast);
+    RuleFactory.createAndAddRule(context, ruleClass, kwargs, ast, env.getStackTrace());
   }
 
   private static RuleClass getBuiltInRuleClass(String ruleClassName, RuleFactory ruleFactory) {
@@ -942,13 +943,13 @@ public final class PackageFactory {
   private static BuiltinFunction newRuleFunction(
       final RuleFactory ruleFactory, final String ruleClass) {
     return new BuiltinFunction(ruleClass, FunctionSignature.KWARGS, BuiltinFunction.USE_AST_ENV) {
-      @SuppressWarnings("unchecked")
+      @SuppressWarnings({"unchecked", "unused"})
       public Runtime.NoneType invoke(Map<String, Object> kwargs,
           FuncallExpression ast, Environment env)
           throws EvalException {
         env.checkLoadingPhase(ruleClass, ast.getLocation());
         try {
-          addRule(ruleFactory, ruleClass, getContext(env, ast), kwargs, ast);
+          addRule(ruleFactory, ruleClass, getContext(env, ast), kwargs, ast, env);
         } catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) {
           throw new EvalException(ast.getLocation(), e.getMessage());
         }
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 a9408ee754..485bfa8b6c 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,6 +15,7 @@
 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;
@@ -23,6 +24,7 @@ import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
 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 java.util.Map;
 import java.util.Set;
@@ -75,7 +77,8 @@ public class RuleFactory {
       Map<String, Object> attributeValues,
       EventHandler eventHandler,
       FuncallExpression ast,
-      Location location)
+      Location location,
+      ImmutableList<StackTraceElement> stackTrace)
       throws InvalidRuleException, NameConflictException {
     Preconditions.checkNotNull(ruleClass);
     String ruleClassName = ruleClass.getName();
@@ -105,8 +108,9 @@ public class RuleFactory {
     }
 
     try {
-      Rule rule = ruleClass.createRuleWithLabel(pkgBuilder, label, attributeValues,
-          eventHandler, ast, location);
+      Rule rule = ruleClass.createRuleWithLabel(pkgBuilder, label,
+          addGeneratorAttributesForMacros(attributeValues, stackTrace), eventHandler, ast,
+          location);
       return rule;
     } catch (SyntaxException e) {
       throw new RuleFactory.InvalidRuleException(ruleClass + " " + e.getMessage());
@@ -114,7 +118,22 @@ public class RuleFactory {
   }
 
   /**
-   * Creates and returns a rule instance.
+   * 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, NameConflictException {
+    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
    * @param ruleClass the class of the rule; this must not be null
@@ -126,6 +145,8 @@ 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
@@ -135,8 +156,11 @@ public class RuleFactory {
                   Map<String, Object> attributeValues,
                   EventHandler eventHandler,
                   FuncallExpression ast,
-                  Location location) throws InvalidRuleException, NameConflictException {
-    Rule rule = createRule(pkgBuilder, ruleClass, attributeValues, eventHandler, ast, location);
+                  Location location,
+                  ImmutableList<StackTraceElement> stackTrace)
+      throws InvalidRuleException, NameConflictException {
+    Rule rule = createRule(
+        pkgBuilder, ruleClass, attributeValues, eventHandler, ast, location, stackTrace);
     pkgBuilder.addRule(rule);
     return rule;
   }
@@ -144,9 +168,11 @@ public class RuleFactory {
   public static Rule createAndAddRule(PackageContext context,
       RuleClass ruleClass,
       Map<String, Object> attributeValues,
-      FuncallExpression ast) throws InvalidRuleException, NameConflictException {
+      FuncallExpression ast,
+      ImmutableList<StackTraceElement> stackTrace)
+      throws InvalidRuleException, NameConflictException {
     return createAndAddRule(context.pkgBuilder, ruleClass, attributeValues, context.eventHandler,
-        ast, ast.getLocation());
+        ast, ast.getLocation(), stackTrace);
   }
 
   /**
@@ -158,4 +184,36 @@ public class RuleFactory {
       super(message);
     }
   }
+
+  /**
+   * If the rule was created by a macro, this method sets the appropriate values for the
+   * attributes generator_{name, function, location} and returns all attributes.
+   *
+   * <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.
+      return args;
+    }
+
+    StackTraceElement generator = stackTrace.get(0);
+    ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
+    builder.putAll(args);
+    builder.put("generator_name", args.get("name"));
+    builder.put("generator_function", generator.getName());
+    builder.put("generator_location", Location.printPathAndLine(generator.getLocation()));
+
+    try {
+      return builder.build();
+    } catch (IllegalArgumentException ex) {
+      // Just to play it safe.
+      return args;
+    }
+  }
 }
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 4c070b4309..e2540342c7 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
@@ -331,37 +331,14 @@ public class SkylarkRuleClassFunctions {
         }
         RuleClass ruleClass = builder.build(ruleClassName);
         PackageContext pkgContext = (PackageContext) env.lookup(PackageFactory.PKG_CONTEXT);
-        return RuleFactory.createAndAddRule(pkgContext, ruleClass,
-            addGeneratorAttributesForMacros((Map<String, Object>) args[0], env), ast);
+        return RuleFactory.createAndAddRule(
+            pkgContext, ruleClass, (Map<String, Object>) args[0], ast, env.getStackTrace());
       } catch (InvalidRuleException | NameConflictException | NoSuchVariableException e) {
         throw new EvalException(ast.getLocation(), e.getMessage());
       }
     }
 
     /**
-     * If the current rule was created by a macro, this method sets the appropriate values for the
-     * attributes generator_{name, function, location} and returns a map of all attribute values.
-     *
-     * <p>Otherwise, the specified map of arguments is returned without any changes.
-     */
-    private Map<String, Object> addGeneratorAttributesForMacros(
-        Map<String, Object> args, Environment env) {
-      ImmutableList<BaseFunction> stackTrace = env.getStackTrace();
-      if (stackTrace.isEmpty()) {
-        // If there was a macro, it would be on the stack.
-        return args;
-      }
-
-      BaseFunction generator = stackTrace.get(0); // BaseFunction
-      ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
-      builder.putAll(args);
-      builder.put("generator_name", args.get("name"));
-      builder.put("generator_function", generator.getName());
-      builder.put("generator_location", generator.getLocationPathAndLine());
-      return builder.build();
-    }
-
-    /**
      * Export a RuleFunction from a Skylark file with a given name.
      */
     void export(PathFragment skylarkFile, String ruleClassName) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
index 94a4f4c4fd..8d5f48f2e0 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
@@ -21,9 +21,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Ordering;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.events.Location.LineAndColumn;
 import com.google.devtools.build.lib.packages.Type.ConversionException;
-import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -199,7 +197,7 @@ public abstract class BaseFunction {
   /**
    * Process the caller-provided arguments into an array suitable for the callee (this function).
    */
-  public Object[] processArguments(@Nullable List<Object> args,
+  public Object[] processArguments(List<Object> args,
       @Nullable Map<String, Object> kwargs,
       @Nullable Location loc)
       throws EvalException {
@@ -413,26 +411,31 @@ public abstract class BaseFunction {
    * @return the value resulting from evaluating the function with the given arguments
    * @throws construction of EvalException-s containing source information.
    */
-  public Object call(@Nullable List<Object> args,
+  public Object call(List<Object> args,
       @Nullable Map<String, Object> kwargs,
       @Nullable FuncallExpression ast,
       @Nullable Environment parentEnv)
       throws EvalException, InterruptedException {
     Environment env = getOrCreateChildEnvironment(parentEnv);
-    Preconditions.checkState(isConfigured(), "Function %s was not configured", getName());
+    env.addToStackTrace(new StackTraceElement(this, kwargs));
+    try {
+      Preconditions.checkState(isConfigured(), "Function %s was not configured", getName());
 
-    // ast is null when called from Java (as there's no Skylark call site).
-    Location loc = ast == null ? location : ast.getLocation();
+      // ast is null when called from Java (as there's no Skylark call site).
+      Location loc = ast == null ? location : ast.getLocation();
 
-    Object[] arguments = processArguments(args, kwargs, loc);
-    canonicalizeArguments(arguments, loc);
+      Object[] arguments = processArguments(args, kwargs, loc);
+      canonicalizeArguments(arguments, loc);
 
-    try {
-      return call(arguments, ast, env);
-    } catch (EvalExceptionWithStackTrace ex) {
-      throw updateStackTrace(ex, loc);
-    } catch (EvalException | RuntimeException | InterruptedException ex) {
-      throw updateStackTrace(new EvalExceptionWithStackTrace(ex, loc), loc);
+      try {
+        return call(arguments, ast, env);
+      } catch (EvalExceptionWithStackTrace ex) {
+        throw updateStackTrace(ex, loc);
+      } catch (EvalException | RuntimeException | InterruptedException ex) {
+        throw updateStackTrace(new EvalExceptionWithStackTrace(ex, loc), loc);
+      }
+    } finally {
+      env.removeStackTraceElement();
     }
   }
 
@@ -574,26 +577,8 @@ public abstract class BaseFunction {
     return Objects.hash(name, location);
   }
 
-  /**
-   * Returns the location (filename:line) of the BaseFunction's definition.
-   *
-   * <p>If such a location is not defined, this method returns an empty string.
-   */
-  public String getLocationPathAndLine() {
-    if (location == null) {
-      return "";
-    }
-
-    StringBuilder builder = new StringBuilder();
-    PathFragment path = location.getPath();
-    if (path != null) {
-      builder.append(path.getPathString());
-    }
-
-    LineAndColumn position = location.getStartLineAndColumn();
-    if (position != null) {
-      builder.append(":").append(position.getLine());
-    }
-    return builder.toString();
+  @Nullable
+  public Location getLocation() {
+    return location;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index f16d35ec82..ed30671273 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -23,8 +23,10 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -104,22 +106,40 @@ public class Environment {
   @Nullable protected EventHandler eventHandler;
 
   /**
+   * A stack trace containing the current history of functions and the calling rule.
+   *
+   * <p>For the rule, the stack trace has two elements: one for the call to the rule in the BUILD
+   * file and one for the actual rule implementation.
+   */
+  private Deque<StackTraceElement> stackTrace;
+
+  /**
    * Constructs an empty root non-Skylark environment.
    * The root environment is also the global environment.
    */
-  public Environment() {
+  public Environment(Deque<StackTraceElement> stackTrace) {
     this.parent = null;
     this.importedExtensions = new HashMap<>();
+    this.stackTrace = stackTrace;
     setupGlobal();
   }
 
+  public Environment() {
+    this(new LinkedList<StackTraceElement>());
+  }
+
   /**
    * Constructs an empty child environment.
    */
-  public Environment(Environment parent) {
+  public Environment(Environment parent, Deque<StackTraceElement> stackTrace) {
     Preconditions.checkNotNull(parent);
     this.parent = parent;
     this.importedExtensions = new HashMap<>();
+    this.stackTrace = stackTrace;
+  }
+
+  public Environment(Environment parent) {
+    this(parent, new LinkedList<StackTraceElement>());
   }
 
   /**
@@ -301,12 +321,60 @@ public class Environment {
     update(symbol.getName(), value);
   }
 
+  public ImmutableList<StackTraceElement> getStackTrace() {
+    return ImmutableList.copyOf(stackTrace);
+  }
+
+  protected Deque<StackTraceElement> getCopyOfStackTrace() {
+    return new LinkedList<>(stackTrace);
+  }
+
+  /**
+   * Adds the given element to the stack trace (iff the stack is empty) and returns whether it was
+   * successful.
+   */
+  public boolean tryAddingStackTraceRoot(StackTraceElement element) {
+    if (stackTrace.isEmpty()) {
+      stackTrace.add(element);
+      return true;
+    }
+    return false;
+  }
+  
+  public void addToStackTrace(StackTraceElement element)    {
+    stackTrace.add(element);
+  }
+
   /**
-   * Return the current stack trace (list of functions).
+   * Removes the only remaining element from the stack trace.
+   *
+   * <p>This particular element describes the outer-most calling function (usually a rule).
+   *
+   * <p> This method is required since {@link FuncallExpression} does not create a new {@link
+   * Environment}, hence it has to add and remove its {@link StackTraceElement} from an existing
+   * one.
    */
-  public ImmutableList<BaseFunction> getStackTrace() {
-    // Empty list, since this environment does not allow function definition
-    // (see SkylarkEnvironment)
-    return ImmutableList.of();
+  public void removeStackTraceRoot() {
+    Preconditions.checkArgument(stackTrace.size() == 1);
+    stackTrace.clear();
+  }
+
+  public void removeStackTraceElement() {
+    // TODO(fwe): find out why the precond doesn't work
+    //    Preconditions.checkArgument(stackTrace.size() > 1);
+    stackTrace.removeLast();
+  }
+
+  /**
+   * Returns whether the given {@link BaseFunction} is part of this {@link Environment}'s stack
+   * trace.
+   */
+  public boolean stackTraceContains(BaseFunction function) {
+    for (StackTraceElement element : stackTrace) {
+      if (element.hasFunction(function)) {
+        return true;
+      }
+    }
+    return false;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
index 686ccab083..82ca835ccd 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
@@ -232,6 +232,7 @@ public class EvalExceptionWithStackTrace extends EvalException {
    * <p> If the exception itself does not have a message, a new message is constructed from the
    * exception's class name.
    * For example, an IllegalArgumentException will lead to "Illegal Argument".
+   * Additionally, the location in the Java code will be added, if applicable,
    */
   private static String getNonEmptyMessage(Exception original) {
     Preconditions.checkNotNull(original);
@@ -252,6 +253,18 @@ public class EvalExceptionWithStackTrace extends EvalException {
       first = false;
     }
 
+    java.lang.StackTraceElement[] trace = original.getStackTrace();
+    if (trace.length > 0) {
+      builder.append(String.format(": %s.%s() in %s:%d", getShortClassName(trace[0]),
+          trace[0].getMethodName(), trace[0].getFileName(), trace[0].getLineNumber()));
+    }
+
     return builder.toString();
   }
+
+  private static String getShortClassName(java.lang.StackTraceElement element) {
+    String name = element.getClassName();
+    int pos = name.lastIndexOf('.');
+    return (pos < 0) ? name : name.substring(pos + 1);
+  }
 }
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 e09dc2aa89..8d1eb56f9c 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
@@ -490,7 +490,20 @@ public final class FuncallExpression extends Expression {
 
   @Override
   Object eval(Environment env) throws EvalException, InterruptedException {
-    return (obj != null) ? invokeObjectMethod(env) : invokeGlobalFunction(env);
+    // Adds the calling rule to the stack trace of the Environment if it is a BUILD environment.
+    // There are two reasons for this:
+    // a) When using aliases in load(), the rule class name in the BUILD file will differ from
+    //    the implementation name in the bzl file. Consequently, we need to store the calling name.
+    // b) We need the location of the calling rule inside the BUILD file.
+    boolean hasAddedElement =
+        env.isSkylark() ? false : env.tryAddingStackTraceRoot(new StackTraceElement(func, args));
+    try {
+      return (obj != null) ? invokeObjectMethod(env) : invokeGlobalFunction(env);
+    } finally {
+      if (hasAddedElement) {
+        env.removeStackTraceRoot();
+      }
+    }
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java
index bbf8be7d4c..0ff19f90c4 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java
@@ -15,14 +15,15 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.util.Fingerprint;
 
 import java.io.Serializable;
+import java.util.Deque;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.Map.Entry;
 import java.util.Set;
 
@@ -41,8 +42,6 @@ public class SkylarkEnvironment extends Environment implements Serializable {
    */
   private final Set<String> readGlobalVariables = new HashSet<>();
 
-  private ImmutableList<BaseFunction> stackTrace;
-
   @Nullable private String fileContentHashCode;
 
   /**
@@ -52,20 +51,17 @@ public class SkylarkEnvironment extends Environment implements Serializable {
   public static SkylarkEnvironment createEnvironmentForFunctionCalling(
       Environment callerEnv, SkylarkEnvironment definitionEnv,
       UserDefinedFunction function) throws EvalException {
-    if (callerEnv.getStackTrace().contains(function)) {
+    if (callerEnv.stackTraceContains(function)) {
       throw new EvalException(
           function.getLocation(),
           "Recursion was detected when calling '" + function.getName() + "' from '"
           + Iterables.getLast(callerEnv.getStackTrace()).getName() + "'");
     }
-    ImmutableList<BaseFunction> stackTrace = new ImmutableList.Builder<BaseFunction>()
-        .addAll(callerEnv.getStackTrace())
-        .add(function)
-        .build();
     SkylarkEnvironment childEnv =
         // Always use the caller Environment's EventHandler. We cannot assume that the
         // definition Environment's EventHandler is still working properly.
-        new SkylarkEnvironment(definitionEnv, stackTrace, callerEnv.eventHandler);
+        new SkylarkEnvironment(
+            definitionEnv, callerEnv.getCopyOfStackTrace(), callerEnv.eventHandler);
     if (callerEnv.isLoadingPhase()) {
       childEnv.setLoadingPhase();
     }
@@ -82,9 +78,8 @@ public class SkylarkEnvironment extends Environment implements Serializable {
   }
 
   private SkylarkEnvironment(SkylarkEnvironment definitionEnv,
-      ImmutableList<BaseFunction> stackTrace, EventHandler eventHandler) {
-    super(definitionEnv.getGlobalEnvironment());
-    this.stackTrace = stackTrace;
+      Deque<StackTraceElement> stackTrace, EventHandler eventHandler) {
+    super(definitionEnv.getGlobalEnvironment(), stackTrace);
     this.eventHandler = Preconditions.checkNotNull(eventHandler,
         "EventHandler cannot be null in an Environment which calls into Skylark");
   }
@@ -92,13 +87,17 @@ public class SkylarkEnvironment extends Environment implements Serializable {
   /**
    * Creates a global SkylarkEnvironment.
    */
-  public SkylarkEnvironment(EventHandler eventHandler, String astFileContentHashCode) {
-    super();
-    stackTrace = ImmutableList.of();
+  public SkylarkEnvironment(EventHandler eventHandler, String astFileContentHashCode,
+      Deque<StackTraceElement> stackTrace) {
+    super(stackTrace);
     this.eventHandler = eventHandler;
     this.fileContentHashCode = astFileContentHashCode;
   }
 
+  public SkylarkEnvironment(EventHandler eventHandler, String astFileContentHashCode) {
+    this(eventHandler, astFileContentHashCode, new LinkedList<StackTraceElement>());
+  }
+
   @VisibleForTesting
   public SkylarkEnvironment(EventHandler eventHandler) {
     this(eventHandler, null);
@@ -106,21 +105,16 @@ public class SkylarkEnvironment extends Environment implements Serializable {
 
   public SkylarkEnvironment(SkylarkEnvironment globalEnv) {
     super(globalEnv);
-    stackTrace = ImmutableList.of();
     this.eventHandler = globalEnv.eventHandler;
   }
 
-  @Override
-  public ImmutableList<BaseFunction> getStackTrace() {
-    return stackTrace;
-  }
-
   /**
    * Clones this Skylark global environment.
    */
   public SkylarkEnvironment cloneEnv(EventHandler eventHandler) {
     Preconditions.checkArgument(isGlobal());
-    SkylarkEnvironment newEnv = new SkylarkEnvironment(eventHandler, this.fileContentHashCode);
+    SkylarkEnvironment newEnv =
+        new SkylarkEnvironment(eventHandler, this.fileContentHashCode, getCopyOfStackTrace());
     for (Entry<String, Object> entry : env.entrySet()) {
       newEnv.env.put(entry.getKey(), entry.getValue());
     }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StackTraceElement.java b/src/main/java/com/google/devtools/build/lib/syntax/StackTraceElement.java
new file mode 100644
index 0000000000..0640ed6387
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StackTraceElement.java
@@ -0,0 +1,95 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.syntax;
+
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.syntax.Argument.Passed;
+
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Represents an element of {@link Environment}'s stack trace.
+ */
+// TODO(fwe): maybe combine this with EvalExceptionWithStackTrace.StackTraceElement
+public final class StackTraceElement {
+  private final Location location;
+  private final String name;
+  private final String nameArg;
+  private final BaseFunction func;
+
+  public StackTraceElement(BaseFunction func, Map<String, Object> kwargs) {
+    this(func.getName(), func.getLocation(), func, getNameArg(kwargs));
+  }
+
+  public StackTraceElement(Identifier identifier, List<Passed> args) {
+    this(identifier.getName(), identifier.getLocation(), null, getNameArg(args));
+  }
+
+  private StackTraceElement(String name, Location location, BaseFunction func, String nameArg) {
+    this.name = name;
+    this.location = location;
+    this.func = func;
+    this.nameArg = nameArg;
+  }
+
+  /**
+   * Returns the value of the argument 'name' (or null if there is none).
+   */
+  private static String getNameArg(Map<String, Object> kwargs) {
+    Object value = (kwargs == null) ? null : kwargs.get("name");
+    return (value == null) ? null : value.toString();
+  }
+
+  /**
+   * Returns the value of the argument 'name' (or null if there is none).
+   */
+  private static String getNameArg(List<Passed> args) {
+    for (Argument.Passed arg : args) {
+      if (arg != null) {
+        String name = arg.getName();
+        if (name != null && name.equals("name")) {
+          Expression expr = arg.getValue();
+          return (expr == null) ? null : expr.toString();
+        }
+      }
+    }
+    return null;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public Location getLocation() {
+    return location;
+  }
+
+  /**
+   * Returns a more expressive description of this element, if possible.
+   */
+  public String getLabel() {
+    return (nameArg == null) ? getName() : String.format("%s(name = %s)", name, nameArg);
+  }
+
+  public boolean hasFunction(BaseFunction func) {
+    return this.func != null && this.func.equals(func);
+  }
+
+  @Override
+  public String toString() {
+    return String.format(
+        "%s @ %s", getLabel(), (location == null) ? "<unknown>" : location.toString());
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
index 360688dc85..3e37c009ad 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 
@@ -44,10 +43,6 @@ public class UserDefinedFunction extends BaseFunction {
     return statements;
   }
 
-  Location getLocation() {
-    return location;
-  }
-
   @Override
   public Object call(Object[] arguments, FuncallExpression ast, Environment env)
       throws EvalException, InterruptedException {
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ExceptionTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ExceptionTest.java
index b737106c39..dd2e83156a 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ExceptionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ExceptionTest.java
@@ -31,7 +31,8 @@ public class ExceptionTest {
   public void testEmptyMessage() throws Exception {
     EvalExceptionWithStackTrace ex =
         new EvalExceptionWithStackTrace(new NullPointerException(), Location.BUILTIN);
-    assertThat(ex).hasMessage("Null Pointer");
+    assertThat(ex.getMessage())
+        .contains("Null Pointer: ExceptionTest.testEmptyMessage() in ExceptionTest.java:");
   }
 
   @Test