Mark ASTFileLookupValue as a NotComparableSkyValue. See the added comments for motivation.

RELNOTES: None
PiperOrigin-RevId: 190794479

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();