diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/syntax')
7 files changed, 61 insertions, 123 deletions
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 b7f562027b..43d27a2147 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,7 +21,9 @@ 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; @@ -59,7 +61,7 @@ import javax.annotation.Nullable; // Provide optimized argument frobbing depending of FunctionSignature and CallerSignature // (that FuncallExpression must supply), optimizing for the all-positional and all-keyword cases. // Also, use better pure maps to minimize map O(n) re-creation events when processing keyword maps. -public abstract class BaseFunction implements StackTraceElement { +public abstract class BaseFunction { // The name of the function private final String name; @@ -98,7 +100,6 @@ public abstract class BaseFunction implements StackTraceElement { /** Returns the name of this function. */ - @Override public String getName() { return name; } @@ -213,7 +214,7 @@ public abstract class BaseFunction implements StackTraceElement { // Note that this variable will be adjusted down if there are extra positionals, // after these extra positionals are dumped into starParam. - int numPositionalArgs = (args == null) ? 0 : args.size(); + int numPositionalArgs = args.size(); int numMandatoryPositionalParams = shape.getMandatoryPositionals(); int numOptionalPositionalParams = shape.getOptionalPositionals(); @@ -415,8 +416,8 @@ public abstract class BaseFunction implements StackTraceElement { @Nullable Environment parentEnv) throws EvalException, InterruptedException { Environment env = getOrCreateChildEnvironment(parentEnv); - 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(); @@ -570,9 +571,26 @@ public abstract class BaseFunction implements StackTraceElement { return Objects.hash(name, location); } - @Override - @Nullable - public Location getLocation() { - return 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(); } } 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 f46538b4c1..3b4342a5e5 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 @@ -142,35 +142,23 @@ public class Environment { */ @Nullable protected EventHandler eventHandler; - private ImmutableList<StackTraceElement> stackTrace; - /** * Constructs an empty root non-Skylark environment. * The root environment is also the global environment. */ - public Environment(ImmutableList<StackTraceElement> stackTrace) { + public Environment() { this.parent = null; this.importedExtensions = new HashMap<>(); - this.stackTrace = stackTrace; setupGlobal(); } - public Environment() { - this(ImmutableList.<StackTraceElement>of()); - } - /** * Constructs an empty child environment. */ - public Environment(Environment parent, ImmutableList<StackTraceElement> stackTrace) { + public Environment(Environment parent) { Preconditions.checkNotNull(parent); this.parent = parent; this.importedExtensions = new HashMap<>(); - this.stackTrace = stackTrace; - } - - public Environment(Environment parent) { - this(parent, ImmutableList.<StackTraceElement>of()); } /** @@ -400,53 +388,12 @@ public class Environment { return nameSpaceFunctions != null ? nameSpaceFunctions.keySet() : ImmutableSet.<String>of(); } - public ImmutableList<StackTraceElement> getStackTrace() { - return 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 = ImmutableList.of(element); - return true; - } - return false; - } - - /** - * 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 void removeStackTraceRoot() { - Preconditions.checkArgument(stackTrace.size() == 1); - stackTrace = ImmutableList.of(); - } - - /** - * 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.equals(function)) { - return true; - } - } - return false; - } - /** - * Returns a copy of this {@link Environment}'s stack trace, including the specified element. + * Return the current stack trace (list of functions). */ - protected ImmutableList<StackTraceElement> getCopyOfUpdatedStackTrace(StackTraceElement toAdd) { - return new ImmutableList.Builder<StackTraceElement>().addAll(stackTrace).add(toAdd).build(); + public ImmutableList<BaseFunction> getStackTrace() { + // Empty list, since this environment does not allow function definition + // (see SkylarkEnvironment) + return ImmutableList.of(); } } 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 623706500c..2e90afd3f9 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 @@ -481,19 +481,7 @@ public final class FuncallExpression extends Expression { @Override Object eval(Environment env) throws EvalException, InterruptedException { - // 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(func); - try { - return (obj != null) ? invokeObjectMethod(env) : invokeGlobalFunction(env); - } finally { - if (hasAddedElement) { - env.removeStackTraceRoot(); - } - } + return (obj != null) ? invokeObjectMethod(env) : invokeGlobalFunction(env); } /** diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java index 66a09f0912..ce68d66d60 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java @@ -26,7 +26,7 @@ import javax.annotation.Nullable; /** * Syntax node for an identifier. */ -public final class Identifier extends Expression implements StackTraceElement { +public final class Identifier extends Expression { private final String name; @@ -37,7 +37,6 @@ public final class Identifier extends Expression implements StackTraceElement { /** * Returns the name of the Identifier. */ - @Override public String getName() { return name; } 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 a032b473ec..cc238179d8 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 @@ -42,6 +42,8 @@ public class SkylarkEnvironment extends Environment implements Serializable { */ private final Set<String> readGlobalVariables = new HashSet<>(); + private ImmutableList<BaseFunction> stackTrace; + @Nullable private String fileContentHashCode; /** @@ -51,17 +53,20 @@ public class SkylarkEnvironment extends Environment implements Serializable { public static SkylarkEnvironment createEnvironmentForFunctionCalling( Environment callerEnv, SkylarkEnvironment definitionEnv, UserDefinedFunction function) throws EvalException { - if (callerEnv.stackTraceContains(function)) { + if (callerEnv.getStackTrace().contains(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, callerEnv.getCopyOfUpdatedStackTrace(function), callerEnv.eventHandler); + new SkylarkEnvironment(definitionEnv, stackTrace, callerEnv.eventHandler); if (callerEnv.isLoadingPhase()) { childEnv.setLoadingPhase(); } @@ -78,8 +83,9 @@ public class SkylarkEnvironment extends Environment implements Serializable { } private SkylarkEnvironment(SkylarkEnvironment definitionEnv, - ImmutableList<StackTraceElement> stackTrace, EventHandler eventHandler) { - super(definitionEnv.getGlobalEnvironment(), stackTrace); + ImmutableList<BaseFunction> stackTrace, EventHandler eventHandler) { + super(definitionEnv.getGlobalEnvironment()); + this.stackTrace = stackTrace; this.eventHandler = Preconditions.checkNotNull(eventHandler, "EventHandler cannot be null in an Environment which calls into Skylark"); } @@ -87,17 +93,13 @@ public class SkylarkEnvironment extends Environment implements Serializable { /** * Creates a global SkylarkEnvironment. */ - public SkylarkEnvironment(EventHandler eventHandler, String astFileContentHashCode, - ImmutableList<StackTraceElement> stackTrace) { - super(stackTrace); + public SkylarkEnvironment(EventHandler eventHandler, String astFileContentHashCode) { + super(); + stackTrace = ImmutableList.of(); this.eventHandler = eventHandler; this.fileContentHashCode = astFileContentHashCode; } - public SkylarkEnvironment(EventHandler eventHandler, String astFileContentHashCode) { - this(eventHandler, astFileContentHashCode, ImmutableList.<StackTraceElement>of()); - } - @VisibleForTesting public SkylarkEnvironment(EventHandler eventHandler) { this(eventHandler, null); @@ -105,16 +107,21 @@ 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, getStackTrace()); + SkylarkEnvironment newEnv = new SkylarkEnvironment(eventHandler, this.fileContentHashCode); 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 deleted file mode 100644 index dbec94369b..0000000000 --- a/src/main/java/com/google/devtools/build/lib/syntax/StackTraceElement.java +++ /dev/null @@ -1,26 +0,0 @@ -// 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; - -/** - * Represents an element of {@link Environment}'s stack trace. - */ -// TODO(fwe): maybe combine this with EvalExceptionWithStackTrace.StackTraceElement -public interface StackTraceElement { - String getName(); - - Location getLocation(); -} 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 6ac438b193..f9719ae15d 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,6 +14,7 @@ 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; @@ -43,6 +44,10 @@ public class UserDefinedFunction extends BaseFunction { return statements; } + Location getLocation() { + return location; + } + @Override public Object call(Object[] arguments, FuncallExpression ast, Environment env) throws EvalException, InterruptedException { |