From 4994cb35f634b18a21b85fe16fbaaed4abd0a36e Mon Sep 17 00:00:00 2001 From: Vladimir Moskva Date: Wed, 10 Aug 2016 15:44:31 +0000 Subject: Substituted NoSuchVariableException with manual checks for performance purposes -- MOS_MIGRATED_REVID=129869968 --- .../build/lib/packages/PackageFactory.java | 7 ++- .../build/lib/rules/SkylarkRuleClassFunctions.java | 36 +++++++-------- .../devtools/build/lib/syntax/Environment.java | 51 +++++++--------------- .../devtools/build/lib/syntax/Identifier.java | 6 +-- .../devtools/build/lib/syntax/LoadStatement.java | 2 +- .../build/lib/syntax/compiler/Variable.java | 7 ++- 6 files changed, 41 insertions(+), 68 deletions(-) (limited to 'src/main/java/com/google/devtools') 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 df26e80295..9c1a5c8a0c 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 @@ -44,7 +44,6 @@ import com.google.devtools.build.lib.syntax.BuiltinFunction; import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.Extension; -import com.google.devtools.build.lib.syntax.Environment.NoSuchVariableException; import com.google.devtools.build.lib.syntax.Environment.Phase; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.EvalUtils; @@ -1191,15 +1190,15 @@ public final class PackageFactory { */ public static PackageContext getContext(Environment env, FuncallExpression ast) throws EvalException { - try { - return (PackageContext) env.lookup(PKG_CONTEXT); - } catch (NoSuchVariableException e) { + PackageContext value = (PackageContext) env.lookup(PKG_CONTEXT); + if (value == null) { // if PKG_CONTEXT is missing, we're not called from a BUILD file. This happens if someone // uses native.some_func() in the wrong place. throw new EvalException(ast.getLocation(), "The native module cannot be accessed from here. " + "Wrap the function in a macro and call it from a BUILD file"); } + return value; } /** 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 9433807410..8b6f05230a 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 @@ -73,7 +73,6 @@ import com.google.devtools.build.lib.syntax.BaseFunction; import com.google.devtools.build.lib.syntax.BuiltinFunction; import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.Environment; -import com.google.devtools.build.lib.syntax.Environment.NoSuchVariableException; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.EvalUtils; import com.google.devtools.build.lib.syntax.FuncallExpression; @@ -525,6 +524,11 @@ public class SkylarkRuleClassFunctions { new BuildLangTypedAttributeValuesMap((Map) args[0]); try { PackageContext pkgContext = (PackageContext) env.lookup(PackageFactory.PKG_CONTEXT); + if (pkgContext == null) { + throw new EvalException(ast.getLocation(), + "Cannot instantiate a rule when loading a .bzl file. Rules can only called from " + + "a BUILD file (possibly via a macro)."); + } return RuleFactory.createAndAddRule( pkgContext, ruleClass, @@ -534,11 +538,6 @@ public class SkylarkRuleClassFunctions { pkgContext.getAttributeContainerFactory().apply(ruleClass)); } catch (InvalidRuleException | NameConflictException e) { throw new EvalException(ast.getLocation(), e.getMessage()); - } catch (NoSuchVariableException e) { - // Thrown when trying to get PackageContext. - throw new EvalException(ast.getLocation(), - "Cannot instantiate a rule when loading a .bzl file. Rules can only called from " - + "a BUILD file (possibly via a macro)."); } } @@ -578,11 +577,9 @@ public class SkylarkRuleClassFunctions { // Export aspects first since rules can depend on aspects. for (String name : globalNames) { - Object value; - try { - value = env.lookup(name); - } catch (NoSuchVariableException e) { - throw new AssertionError(e); + Object value = env.lookup(name); + if (name == null) { + throw new AssertionError(String.format("No such variable: '%s'", name)); } if (value instanceof SkylarkAspect) { SkylarkAspect skylarkAspect = (SkylarkAspect) value; @@ -593,16 +590,15 @@ public class SkylarkRuleClassFunctions { } for (String name : globalNames) { - try { - Object value = env.lookup(name); - if (value instanceof RuleFunction) { - RuleFunction function = (RuleFunction) value; - if (function.skylarkLabel == null) { - function.export(skylarkLabel, name); - } + Object value = env.lookup(name); + if (value == null) { + throw new AssertionError(String.format("No such variable: '%s'", name)); + } + if (value instanceof RuleFunction) { + RuleFunction function = (RuleFunction) value; + if (function.skylarkLabel == null) { + function.export(skylarkLabel, name); } - } catch (NoSuchVariableException e) { - throw new AssertionError(e); } } } 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 e9d7bd93dd..ec504a06ec 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 @@ -699,12 +699,7 @@ public final class Environment implements Freezable { } private boolean hasVariable(String varname) { - try { - lookup(varname); - return true; - } catch (NoSuchVariableException e) { - return false; - } + return lookup(varname) != null; } /** @@ -737,10 +732,9 @@ public final class Environment implements Freezable { } /** - * @return the value from the environment whose name is "varname". - * @throws NoSuchVariableException if the variable is not defined in the Environment. + * @return the value from the environment whose name is "varname" if it exists, otherwise null. */ - public Object lookup(String varname) throws NoSuchVariableException { + public Object lookup(String varname) { // Which Frame to lookup first doesn't matter because update prevents clashes. if (lexicalFrame != null) { Object lexicalValue = lexicalFrame.get(varname); @@ -751,7 +745,7 @@ public final class Environment implements Freezable { Object globalValue = globalFrame.get(varname); Object dynamicValue = dynamicFrame.get(varname); if (globalValue == null && dynamicValue == null) { - throw new NoSuchVariableException(varname); + return null; } if (knownGlobalVariables != null) { knownGlobalVariables.add(varname); @@ -768,11 +762,11 @@ public final class Environment implements Freezable { */ public Object lookup(String varname, Object defaultValue) { Preconditions.checkState(!isSkylark); - try { - return lookup(varname); - } catch (NoSuchVariableException e) { - return defaultValue; + Object value = lookup(varname); + if (value != null) { + return value; } + return defaultValue; } /** @@ -817,23 +811,6 @@ public final class Environment implements Freezable { return String.format("", mutability()); } - /** - * An Exception thrown when an attempt is made to lookup a non-existent - * variable in the Environment. - */ - public static class NoSuchVariableException extends Exception { - private final String variable; - NoSuchVariableException(String variable) { - super(null, null, false, false /* don't fillInStackTrace() */); - this.variable = variable; - } - - @Override - public String getMessage() { - return "no such variable: " + variable; - } - } - /** * An Exception thrown when an attempt is made to import a symbol from a file * that was not properly loaded. @@ -842,12 +819,16 @@ public final class Environment implements Freezable { LoadFailedException(String importString) { super(String.format("file '%s' was not correctly loaded. " + "Make sure the 'load' statement appears in the global scope in your file", - importString)); + importString)); + } + + LoadFailedException(String importString, String symbolString) { + super(String.format("file '%s' does not contain symbol '%s'", importString, symbolString)); } } void importSymbol(String importString, Identifier symbol, String nameInLoadedFile) - throws NoSuchVariableException, LoadFailedException { + throws LoadFailedException { Preconditions.checkState(isGlobal()); // loading is only allowed at global scope. if (!importedExtensions.containsKey(importString)) { @@ -856,10 +837,8 @@ public final class Environment implements Freezable { Extension ext = importedExtensions.get(importString); - // TODO(bazel-team): Throw a LoadFailedException instead, with an appropriate message. - // Throwing a NoSuchVariableException is backward compatible, but backward. if (!ext.containsKey(nameInLoadedFile)) { - throw new NoSuchVariableException(nameInLoadedFile); + throw new LoadFailedException(importString, nameInLoadedFile); } Object value = ext.get(nameInLoadedFile); 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 0f1fc18a6f..f953b05d08 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 @@ -72,11 +72,11 @@ public final class Identifier extends Expression { @Override Object doEval(Environment env) throws EvalException { - try { - return env.lookup(name); - } catch (Environment.NoSuchVariableException e) { + Object value = env.lookup(name); + if (value == null) { throw createInvalidIdentifierException(); } + return value; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java index b1e6c1579a..35fb5d1784 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java @@ -82,7 +82,7 @@ public final class LoadStatement extends Statement { // The key is the original name that was used to define the symbol // in the loaded bzl file. env.importSymbol(imp.getImportString(), name, declared.getName()); - } catch (Environment.NoSuchVariableException | Environment.LoadFailedException e) { + } catch (Environment.LoadFailedException e) { throw new EvalException(getLocation(), e.getMessage()); } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/compiler/Variable.java b/src/main/java/com/google/devtools/build/lib/syntax/compiler/Variable.java index 41e2e5d32d..39dbe7b987 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/compiler/Variable.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/compiler/Variable.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.syntax.compiler; import com.google.devtools.build.lib.syntax.ASTNode; import com.google.devtools.build.lib.syntax.Environment; -import com.google.devtools.build.lib.syntax.Environment.NoSuchVariableException; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.EvalExceptionWithStackTrace; import com.google.devtools.build.lib.syntax.compiler.DebugInfo.AstAccessors; @@ -107,15 +106,15 @@ public abstract class Variable { */ public static Object lookupUnboundVariable(Environment global, String variable, ASTNode node) throws EvalExceptionWithStackTrace { - try { - return global.lookup(variable); - } catch (NoSuchVariableException e) { + Object value = global.lookup(variable); + if (value == null) { throw new EvalExceptionWithStackTrace( new EvalException( node.getLocation(), "local variable '" + variable + "' referenced before assignment"), node); } + return value; } } -- cgit v1.2.3