From 82cfb5582d0f33eb0703b77b32dcf2dd080c07d4 Mon Sep 17 00:00:00 2001 From: nharmata Date: Wed, 28 Mar 2018 10:43:48 -0700 Subject: Mark ASTFileLookupValue as a NotComparableSkyValue. See the added comments for motivation. RELNOTES: None PiperOrigin-RevId: 190794479 --- .../devtools/build/lib/skyframe/ASTFileLookupValue.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java index 247d0e0202..2bb6d1ab9f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java @@ -21,15 +21,25 @@ import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.skyframe.AbstractSkyKey; +import com.google.devtools.build.skyframe.NotComparableSkyValue; import com.google.devtools.build.skyframe.SkyFunctionName; -import com.google.devtools.build.skyframe.SkyValue; /** * A value that represents an AST file lookup result. There are two subclasses: one for the case * where the file is found, and another for the case where the file is missing (but there are no * other errors). */ -public abstract class ASTFileLookupValue implements SkyValue { +// In practice, if a ASTFileLookupValue is re-computed (i.e. not changed pruned), then it will +// almost certainly be unequal to the previous value. This is because of (i) the change-pruning +// semantics of the PackageLookupValue dep and the FileValue dep; consider the latter: if the +// FileValue for the bzl file has changed, then the contents of the bzl file probably changed and +// (ii) we don't currently have skylark-semantic-equality in BuildFileAST, so two BuildFileAST +// instances representing two different contents of a bzl file will be different. +// TODO(bazel-team): Consider doing better here. As a pre-req, we would need +// skylark-semantic-equality in BuildFileAST, rather than equality naively based on the contents of +// the bzl file. For a concrete example, the contents of comment lines do not currently impact +// skylark semantics. +public abstract class ASTFileLookupValue implements NotComparableSkyValue { public abstract boolean lookupSuccessful(); public abstract BuildFileAST getAST(); public abstract String getErrorMsg(); -- cgit v1.2.3