diff options
author | Jon Brandvein <brandjon@google.com> | 2017-03-01 16:01:39 +0000 |
---|---|---|
committer | Yue Gan <yueg@google.com> | 2017-03-01 16:17:01 +0000 |
commit | 34ffc4cab1b6cdde54e5945427b79c24fff39aa5 (patch) | |
tree | 21d7369043d9e0e1e6b7820a47851d32f7751a7a /src/main/java/com | |
parent | 9f15d156cf977c57ec0c88f4aed44b13053768fb (diff) |
Rollback of commit 01120026dc313ee7ad9ea95069a29252eb19173b.
--
PiperOrigin-RevId: 148888469
MOS_MIGRATED_REVID=148888469
Diffstat (limited to 'src/main/java/com')
5 files changed, 95 insertions, 146 deletions
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 4b8c90da63..b28b08e07c 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 @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.events.Location; 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; @@ -273,18 +274,6 @@ public final class SkylarkAttr { "cfg must be either 'data', 'host', or 'target'."); } } - - if (containsNonNoneKey(arguments, ASPECTS_ARG)) { - Object obj = arguments.get(ASPECTS_ARG); - SkylarkType.checkType(obj, SkylarkList.class, ASPECTS_ARG); - - List<SkylarkAspect> aspects = - ((SkylarkList<?>) obj).getContents(SkylarkAspect.class, "aspects"); - for (SkylarkAspect aspect : aspects) { - builder.aspect(aspect, ast.getLocation()); - } - } - return builder; } @@ -641,13 +630,12 @@ public final class SkylarkAttr { SINGLE_FILE_ARG, singleFile, CONFIGURATION_ARG, - cfg, - ASPECTS_ARG, - aspects - ), + cfg), ast, env); - return new Descriptor(attribute); + ImmutableList<SkylarkAspect> skylarkAspects = + ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects")); + return new Descriptor(attribute, skylarkAspects); } catch (EvalException e) { throw new EvalException(ast.getLocation(), e.getMessage(), e); } @@ -920,14 +908,13 @@ public final class SkylarkAttr { ALLOW_EMPTY_ARG, allowEmpty, CONFIGURATION_ARG, - cfg, - ASPECTS_ARG, - aspects - ); + cfg); try { Attribute.Builder<?> attribute = createAttribute(BuildType.LABEL_LIST, kwargs, ast, env); - return new Descriptor(attribute); + ImmutableList<SkylarkAspect> skylarkAspects = + ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects")); + return new Descriptor(attribute, skylarkAspects); } catch (EvalException e) { throw new EvalException(ast.getLocation(), e.getMessage(), e); } @@ -1073,13 +1060,13 @@ public final class SkylarkAttr { ALLOW_EMPTY_ARG, allowEmpty, CONFIGURATION_ARG, - cfg, - ASPECTS_ARG, - aspects); + cfg); try { Attribute.Builder<?> attribute = createAttribute(BuildType.LABEL_KEYED_STRING_DICT, kwargs, ast, env); - return new Descriptor(attribute); + ImmutableList<SkylarkAspect> skylarkAspects = + ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects")); + return new Descriptor(attribute, skylarkAspects); } catch (EvalException e) { throw new EvalException(ast.getLocation(), e.getMessage(), e); } @@ -1438,17 +1425,33 @@ public final class SkylarkAttr { ) public static final class Descriptor { private final Attribute.Builder<?> attributeBuilder; + private final ImmutableList<SkylarkAspect> aspects; /** * This lock guards {@code attributeBuilder} field. * * {@link Attribute.Builder} class is not thread-safe for concurrent modification. + * This class, together with its enclosing {@link SkylarkAttr} class, do not let + * anyone else access the {@code attributeBuilder}, however {@link #exportAspects(Location)} + * method actually modifies the {@code attributeBuilder}. Therefore all read- and write-accesses + * to {@code attributeBuilder} are protected with this lock. + * + * For example, {@link #hasDefault()} method only reads from {@link #attributeBuilder}, + * but we have no guarantee that it is safe to do so concurrently with adding aspects + * in {@link #exportAspects(Location)}. */ private final Object lock = new Object(); + boolean exported; + + private Descriptor(Attribute.Builder<?> attributeBuilder) { + this(attributeBuilder, ImmutableList.<SkylarkAspect>of()); + } private Descriptor( - Attribute.Builder<?> attributeBuilder) { + Attribute.Builder<?> attributeBuilder, ImmutableList<SkylarkAspect> aspects) { this.attributeBuilder = attributeBuilder; + this.aspects = aspects; + exported = false; } public boolean hasDefault() { @@ -1468,6 +1471,27 @@ public final class SkylarkAttr { return attributeBuilder.build(name); } } + + public ImmutableList<SkylarkAspect> getAspects() { + return aspects; + } + + public void exportAspects(Location definitionLocation) throws EvalException { + synchronized (lock) { + if (exported) { + // Only export an attribute definiton once. + return; + } + for (SkylarkAspect skylarkAspect : getAspects()) { + if (!skylarkAspect.isExported()) { + throw new EvalException(definitionLocation, + "All aspects applied to rule dependencies must be top-level values"); + } + attributeBuilder.aspect(skylarkAspect, definitionLocation); + } + exported = true; + } + } } static { 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 5a01e6fd88..3cc9decfe6 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 @@ -94,6 +94,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutionException; /** @@ -683,6 +684,7 @@ public class SkylarkRuleClassFunctions { } for (Pair<String, SkylarkAttr.Descriptor> attribute : attributes) { SkylarkAttr.Descriptor descriptor = attribute.getSecond(); + descriptor.exportAspects(definitionLocation); addAttribute(definitionLocation, builder, descriptor.build(attribute.getFirst())); @@ -718,6 +720,25 @@ public class SkylarkRuleClassFunctions { SkylarkAspect.class, RuleFunction.class); + public static void exportRuleFunctionsAndAspects(Environment env, Label skylarkLabel) + throws EvalException { + Set<String> globalNames = env.getGlobals().getDirectVariableNames(); + + for (Class<? extends SkylarkExportable> exportable : EXPORTABLES) { + for (String name : globalNames) { + Object value = env.lookup(name); + if (value == null) { + throw new AssertionError(String.format("No such variable: '%s'", name)); + } + if (exportable.isInstance(value)) { + if (!exportable.cast(value).isExported()) { + exportable.cast(value).export(skylarkLabel, name); + } + } + } + } + } + @SkylarkSignature( name = "Label", doc = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java index 08c6cf5395..fb68cb2fe6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java @@ -27,21 +27,18 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.BuildFileNotFoundException; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.RuleClassProvider; -import com.google.devtools.build.lib.packages.SkylarkExportable; +import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions; import com.google.devtools.build.lib.skyframe.SkylarkImportLookupValue.SkylarkImportLookupKey; -import com.google.devtools.build.lib.syntax.AssignmentStatement; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.LoadStatement; import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.SkylarkImport; -import com.google.devtools.build.lib.syntax.Statement; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; @@ -370,7 +367,12 @@ public class SkylarkImportLookupFunction implements SkyFunction { .createSkylarkRuleClassEnvironment( extensionLabel, mutability, eventHandler, ast.getContentHashCode(), importMap) .setupOverride("native", packageFactory.getNativeModule(inWorkspace)); - execAndExport(ast, extensionLabel, eventHandler, extensionEnv); + ast.exec(extensionEnv, eventHandler); + try { + SkylarkRuleClassFunctions.exportRuleFunctionsAndAspects(extensionEnv, extensionLabel); + } catch (EvalException e) { + eventHandler.handle(Event.error(e.getLocation(), e.getMessage())); + } Event.replayEventsOn(env.getListener(), eventHandler.getEvents()); if (eventHandler.hasErrors()) { @@ -380,39 +382,6 @@ public class SkylarkImportLookupFunction implements SkyFunction { } } - public static void execAndExport(BuildFileAST ast, Label extensionLabel, - EventHandler eventHandler, - com.google.devtools.build.lib.syntax.Environment extensionEnv) throws InterruptedException { - ImmutableList<Statement> statements = ast.getStatements(); - for (Statement statement : statements) { - ast.execTopLevelStatement(statement, extensionEnv, eventHandler); - possiblyExport(statement, extensionLabel, eventHandler, extensionEnv); - } - } - - private static void possiblyExport(Statement statement, Label extensionLabel, - EventHandler eventHandler, - com.google.devtools.build.lib.syntax.Environment extensionEnv) { - if (!(statement instanceof AssignmentStatement)) { - return; - } - AssignmentStatement assignmentStatement = (AssignmentStatement) statement; - ImmutableSet<String> boundNames = assignmentStatement.getLValue().boundNames(); - for (String name : boundNames) { - Object lookup = extensionEnv.lookup(name); - if (lookup instanceof SkylarkExportable) { - try { - SkylarkExportable exportable = (SkylarkExportable) lookup; - if (!exportable.isExported()) { - exportable.export(extensionLabel, name); - } - } catch (EvalException e) { - eventHandler.handle(Event.error(e.getLocation(), e.getMessage())); - } - } - } - } - @Override public String extractTag(SkyKey skyKey) { return null; diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java index b1f079a14b..a0f6a33367 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java @@ -194,52 +194,27 @@ public class BuildFileAST extends ASTNode { public boolean exec(Environment env, EventHandler eventHandler) throws InterruptedException { boolean ok = true; for (Statement stmt : stmts) { - if (!execTopLevelStatement(stmt, env, eventHandler)) { + try { + stmt.exec(env); + } catch (EvalException e) { ok = false; + // Do not report errors caused by a previous parsing error, as it has already been + // reported. + if (e.isDueToIncompleteAST()) { + continue; + } + // When the exception is raised from another file, report first the location in the + // BUILD file (as it is the most probable cause for the error). + Location exnLoc = e.getLocation(); + Location nodeLoc = stmt.getLocation(); + eventHandler.handle(Event.error( + (exnLoc == null || !nodeLoc.getPath().equals(exnLoc.getPath())) ? nodeLoc : exnLoc, + e.getMessage())); } } return ok; } - /** - * Executes tol-level statement of this build file in a given Environment. - * - * <p>If, for any reason, execution of a statement cannot be completed, an {@link EvalException} - * is thrown by {@link Statement#exec(Environment)}. This exception is caught here and reported - * through reporter. In effect, there is a - * "try/except" block around every top level statement. Such exceptions are not ignored, though: - * they are visible via the return value. Rules declared in a package containing any error - * (including loading-phase semantical errors that cannot be checked here) must also be considered - * "in error". - * - * <p>Note that this method will not affect the value of {@link #containsErrors()}; that refers - * only to lexer/parser errors. - * - * @return true if no error occurred during execution. - */ - - public boolean execTopLevelStatement(Statement stmt, Environment env, - EventHandler eventHandler) throws InterruptedException { - try { - stmt.exec(env); - return true; - } catch (EvalException e) { - // Do not report errors caused by a previous parsing error, as it has already been - // reported. - if (e.isDueToIncompleteAST()) { - return false; - } - // When the exception is raised from another file, report first the location in the - // BUILD file (as it is the most probable cause for the error). - Location exnLoc = e.getLocation(); - Location nodeLoc = stmt.getLocation(); - eventHandler.handle(Event.error( - (exnLoc == null || !nodeLoc.getPath().equals(exnLoc.getPath())) ? nodeLoc : exnLoc, - e.getMessage())); - return false; - } - } - @Override public String toString() { return "BuildFileAST" + getStatements(); @@ -373,20 +348,10 @@ public class BuildFileAST extends ASTNode { @Nullable public static Object eval(Environment env, String... input) throws EvalException, InterruptedException { - BuildFileAST ast = parseAndValidateSkylarkString(env, input); - return ast.eval(env); - } - - /** - * Parses and validates the lines from input and return the the AST - * In case of error during validation, it throws an EvalException. - */ - public static BuildFileAST parseAndValidateSkylarkString(Environment env, String[] input) - throws EvalException { BuildFileAST ast = parseSkylarkString(env.getEventHandler(), input); ValidationEnvironment valid = new ValidationEnvironment(env); valid.validateAst(ast.getStatements()); - return ast; + return ast.eval(env); } /** diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java index ecc72073fe..22d686bf4e 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.syntax; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.util.Preconditions; import java.io.Serializable; @@ -74,35 +73,6 @@ public class LValue implements Serializable { } } - /** - * Returns all names bound by this LValue. - * - * Examples: - * <ul> - * <li><{@code x = ...} binds x.</li> - * <li><{@code x, [y,z] = ..} binds x, y, z.</li> - * <li><{@code x[5] = ..} does not bind any names.</li> - * </ul> - */ - public ImmutableSet<String> boundNames() { - ImmutableSet.Builder<String> result = ImmutableSet.builder(); - collectBoundNames(expr, result); - return result.build(); - } - - private static void collectBoundNames(Expression lhs, ImmutableSet.Builder<String> result) { - if (lhs instanceof Identifier) { - result.add(((Identifier) lhs).getName()); - return; - } - if (lhs instanceof ListLiteral) { - ListLiteral variables = (ListLiteral) lhs; - for (Expression expression : variables.getElements()) { - collectBoundNames(expression, result); - } - } - } - private static void doAssign( Environment env, Location loc, Expression lhs, Object result) throws EvalException, InterruptedException { |