diff options
author | nharmata <nharmata@google.com> | 2018-03-28 10:43:48 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-03-28 10:45:02 -0700 |
commit | 82cfb5582d0f33eb0703b77b32dcf2dd080c07d4 (patch) | |
tree | a98a32d5a9d1bdd07569b9b2bfa6448a3065e3c9 | |
parent | 9ea02e5daff7194c057ccff295e37e8ed9359147 (diff) |
Mark ASTFileLookupValue as a NotComparableSkyValue. See the added comments for motivation.
RELNOTES: None
PiperOrigin-RevId: 190794479
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java | 14 |
1 files changed, 12 insertions, 2 deletions
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(); |