bazel syntax: stop using concurrent.Immutable annotation The @Immutable annotation specifies that it is for documentation only. Historically lib.syntax has violated this contract by using the presence of the annotation as an indication that values of the annotated class should be considered immutable to Starlark (EvalUtils.isImmutable). This change adds an explicit override of StarkarkValue.isImmutable to every class annotated @Immutable. This is more verbose, but clearer and less magical. In particular, it is obvious that isImmutable is inherited, whereas I suspect few people memorized whether the effect of @Immutable was inherited. (It was not.) It also means there is one mechanism, not two, for expressing Starlark immutability. (Longer term, I would like to merge isImmutable into isHashable, because that is isImmutable's only purpose. But things are currently very inconsistent: for example, you can't hash a list, even if frozen, but you can hash a struct containing a frozen list, because the isHashable recursive case checks immutability, not hashability.) This change required a manual audit, combined with some throwaway static and dynamic checks. It is possible that I have missed at least one place, which will manifest as an unexpected "unhashable: foo" error. I will keep an eye out for regressions. (I am sheriff this week.) Also: - Revert the hack added to Immutable that causes the annotations to be retained at run time. - lib.syntax now uses the JSR305 Immutable annotation in the few places where it matters, so it no longer depends on lib.concurrent. RELNOTES: N/A PiperOrigin-RevId: 312085384

diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 5fd6790..bbf51b8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java

@@ -54,6 +54,12 @@
 @Immutable
 @ThreadSafe
 public abstract class AbstractAction extends ActionKeyCacher implements Action, ActionApi {
+
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /**
    * An arbitrary default resource set. We assume that a typical subprocess is single-threaded
    * (i.e., uses one CPU core) and CPU-bound, and uses a small-ish amount of memory. In the past,

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
index 4ad8ae6..2b65c59 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java

@@ -115,6 +115,11 @@
     this.rootType = rootType;
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   public Root getRoot() {
     return root;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index ad2a017..2b14144 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -1571,6 +1571,7 @@
     srcs = ["config/Fragment.java"],
     deps = [
         ":config/build_options",
+        "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/syntax:evaluator",
         "//third_party:jsr305",

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java
index b834eff..80c913c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java

@@ -19,7 +19,6 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
 import com.google.devtools.build.lib.syntax.ClassObject;
-import com.google.devtools.build.lib.syntax.StarlarkValue;
 import javax.annotation.Nullable;
 
 /**
@@ -32,7 +31,7 @@
  * their direct dependencies, only the corresponding {@link TransitiveInfoCollection}s. Also, {@link
  * ConfiguredTarget} objects should not be accessible from the action graph.
  */
-public interface ConfiguredTarget extends TransitiveInfoCollection, ClassObject, StarlarkValue {
+public interface ConfiguredTarget extends TransitiveInfoCollection, ClassObject {
 
   /** All <code>ConfiguredTarget</code>s have a "label" field. */
   String LABEL_FIELD = "label";

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/EmptyConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/EmptyConfiguredTarget.java
index 95dd272..d3b167d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/EmptyConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/EmptyConfiguredTarget.java

@@ -25,7 +25,7 @@
 import javax.annotation.Nullable;
 
 /** A configured target that is empty. */
-@Immutable // (and Starlark-hashable)
+@Immutable
 public class EmptyConfiguredTarget extends AbstractConfiguredTarget {
   public EmptyConfiguredTarget(Label label, BuildConfigurationValue.Key configurationKey) {
     super(label, configurationKey, NestedSetBuilder.emptySet(Order.STABLE_ORDER));

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/FileProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/FileProvider.java
index 227dc3d..9cac204 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/FileProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/FileProvider.java

@@ -40,6 +40,11 @@
     this.filesToBuild = filesToBuild;
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /**
    * Returns the set of artifacts that are the "output" of this rule.
    *

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/FilesToRunProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/FilesToRunProvider.java
index 4c66a12..f75e72a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/FilesToRunProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/FilesToRunProvider.java

@@ -57,6 +57,11 @@
         NestedSetBuilder.create(Order.STABLE_ORDER, artifact), null, artifact);
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /** Returns artifacts needed to run the executable for this target. */
   public NestedSet<Artifact> getFilesToRun() {
     return filesToRun;

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
index 716fac3..6e544d3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java

@@ -141,6 +141,11 @@
     this.outputGroups = outputGroups;
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   @Nullable
   public static OutputGroupInfo get(ProviderCollection collection) {
     return collection.get(SKYLARK_CONSTRUCTOR);

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
index ad76b9c..c765d3c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java

@@ -314,6 +314,11 @@
     this.legacyExternalRunfiles = legacyExternalRunfiles;
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /**
    * Returns the runfiles' suffix.
    */

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/Fragment.java b/src/main/java/com/google/devtools/build/lib/analysis/config/Fragment.java
index 91e938c..61597cc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/Fragment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/Fragment.java

@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis.config;
 
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.syntax.StarlarkValue;
 import java.util.List;
@@ -27,7 +28,14 @@
  *
  * <p>Fragments are Starlark values, as returned by {@code ctx.fragments.android}, for example.
  */
+@Immutable
 public abstract class Fragment implements StarlarkValue {
+
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /**
    * Validates the options for this Fragment. Issues warnings for the use of deprecated options, and
    * warnings or errors for any option settings that conflict.

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentCollection.java
index c337f3a..085fedd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentCollection.java

@@ -35,6 +35,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   @Nullable
   public Object getValue(String name) throws EvalException {
     return ruleContext.getSkylarkFragment(name, transition);

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java
index 412bfbd..9766cbe 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java

@@ -34,7 +34,6 @@
 import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
 import com.google.devtools.build.lib.packages.Provider;
 import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
-import com.google.devtools.build.lib.syntax.ClassObject;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Printer;
 import com.google.devtools.build.lib.syntax.Starlark;
@@ -47,8 +46,7 @@
  * An abstract implementation of ConfiguredTarget in which all properties are assigned trivial
  * default values.
  */
-public abstract class AbstractConfiguredTarget
-    implements ConfiguredTarget, VisibilityProvider, ClassObject {
+public abstract class AbstractConfiguredTarget implements ConfiguredTarget, VisibilityProvider {
   private final Label label;
   private final BuildConfigurationValue.Key configurationKey;
 
@@ -95,6 +93,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // all Targets are immutable and Starlark-hashable
+  }
+
+  @Override
   public final NestedSet<PackageGroupContents> getVisibility() {
     return visibility;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/EnvironmentGroupConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/EnvironmentGroupConfiguredTarget.java
index b95f126..ad3b159 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/EnvironmentGroupConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/EnvironmentGroupConfiguredTarget.java

@@ -27,7 +27,7 @@
  * groups are not really first-class Targets.
  */
 @AutoCodec
-@Immutable // (and Starlark-hashable)
+@Immutable
 public final class EnvironmentGroupConfiguredTarget extends AbstractConfiguredTarget {
   @AutoCodec.Instantiator
   @AutoCodec.VisibleForSerialization

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FileConfiguredTarget.java
index 12b5e9e..853b909 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FileConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FileConfiguredTarget.java

@@ -43,7 +43,7 @@
  * A ConfiguredTarget for a source FileTarget. (Generated files use a subclass,
  * OutputFileConfiguredTarget.)
  */
-@Immutable // (and Starlark-hashable)
+@Immutable
 public abstract class FileConfiguredTarget extends AbstractConfiguredTarget
     implements FileType.HasFileType, LicensesProvider {
 

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java
index c3fcf01..65b0f11 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java

@@ -30,7 +30,6 @@
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Instantiator;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
 import com.google.devtools.build.lib.syntax.Printer;
-import com.google.devtools.build.lib.syntax.StarlarkValue;
 import java.util.Objects;
 
 /**
@@ -40,8 +39,8 @@
  * here and is always set to <b>null</b>.
  */
 @AutoCodec
-@Immutable // (and Starlark-hashable)
-public final class InputFileConfiguredTarget extends FileConfiguredTarget implements StarlarkValue {
+@Immutable
+public final class InputFileConfiguredTarget extends FileConfiguredTarget {
   private final SourceArtifact artifact;
   private final NestedSet<TargetLicense> licenses;
 

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
index a44b180..5d44784 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java

@@ -43,7 +43,7 @@
  * <p>This is an ephemeral object created only for the analysis of a single configured target. After
  * that configured target is analyzed, this is thrown away.
  */
-@Immutable // (and Starlark-hashable)
+@Immutable
 public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
   private final ConfiguredTarget base;
   private final ImmutableList<ConfiguredAspect> aspects;

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/OutputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/OutputFileConfiguredTarget.java
index a01c2c0..95a879c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/OutputFileConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/OutputFileConfiguredTarget.java

@@ -38,7 +38,7 @@
 
 /** A ConfiguredTarget for an OutputFile. */
 @AutoCodec
-@Immutable // (and Starlark-hashable)
+@Immutable
 public class OutputFileConfiguredTarget extends FileConfiguredTarget {
 
   private final Artifact artifact;

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/PackageGroupConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/PackageGroupConfiguredTarget.java
index 76ae3c0..2d58c7f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/PackageGroupConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/PackageGroupConfiguredTarget.java

@@ -40,7 +40,7 @@
  * not really first-class Targets.
  */
 @AutoCodec
-@Immutable // (and Starlark-hashable)
+@Immutable
 public final class PackageGroupConfiguredTarget extends AbstractConfiguredTarget
     implements PackageSpecificationProvider {
   private static final FileProvider NO_FILES = new FileProvider(

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java
index ea951d1..4180582 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java

@@ -59,7 +59,7 @@
  * works, see {@link com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory}.
  */
 @AutoCodec(checkClassExplicitlyAllowed = true)
-@Immutable // (and Starlark-hashable)
+@Immutable
 public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
 
   /** A set of this target's implicitDeps. */

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintCollection.java
index 1309d83..0f196f7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintCollection.java

@@ -107,6 +107,11 @@
     return builder().parent(parent).addConstraints(constraints).build();
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /**
    * Returns the parent {@link ConstraintCollection} for this instance, or {@code null} if none
    * exists.

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/Args.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/Args.java
index 0d3d37d..ebd7f90 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/Args.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/Args.java

@@ -141,6 +141,12 @@
     }
 
     @Override
+    public boolean isImmutable() {
+      return true; // immutable but not directly hashable (though may be hashed as an element of,
+      // say, a struct).
+    }
+
+    @Override
     public ImmutableSet<Artifact> getDirectoryArtifacts() {
       return directoryInputs;
     }

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkExecutionResult.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkExecutionResult.java
index a199ae8..db89b9c0 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkExecutionResult.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkExecutionResult.java

@@ -54,6 +54,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public int getReturnCode() {
     return returnCode;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkOS.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkOS.java
index daf8435..c4135e7 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkOS.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkOS.java

@@ -30,6 +30,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public ImmutableMap<String, String> getEnvironmentVariables() {
     return environ;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkPath.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkPath.java
index 98b80d9..1986ce6 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkPath.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/StarlarkPath.java

@@ -40,6 +40,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public boolean equals(Object obj) {
     return (obj instanceof StarlarkPath) &&  path.equals(((StarlarkPath) obj).path);
   }

diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java
index fe4b466..0f0a125 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java

@@ -49,7 +49,6 @@
  *
  * <p>Every call to {@code depset} returns a distinct instance equal to no other.
  */
-// TODO(adonovan): move to lib.packages, as this is a Bazelism.
 @StarlarkBuiltin(
     name = "depset",
     category = StarlarkDocumentationCategory.BUILTIN,

diff --git a/src/main/java/com/google/devtools/build/lib/concurrent/ThreadSafety.java b/src/main/java/com/google/devtools/build/lib/concurrent/ThreadSafety.java
index 4488aec..329c343 100644
--- a/src/main/java/com/google/devtools/build/lib/concurrent/ThreadSafety.java
+++ b/src/main/java/com/google/devtools/build/lib/concurrent/ThreadSafety.java

@@ -21,33 +21,28 @@
 
 /**
  * Define some standard attributes for documenting thread safety properties.
- *<p>
- * The names used here are adapted from those used in Joshua Bloch's book
- * "Effective Java", which are also described at
- * <http://www.ibm.com/developerworks/java/library/j-jtp09263/>.
- *<p>
- * These attributes are just documentation.  They don't have any run-time
- * effect.  The main aim is mainly just to standardize the terminology.
- * (However, if this catches on, I can also imagine in the future having
- * a presubmit check that checks that all new classes have thread safety
- * annotations :)
- *<p>
- * See ThreadSafetyTest for examples of how these attributes should be used.
+ *
+ * <p>The names used here are adapted from those used in Joshua Bloch's book "Effective Java", which
+ * are also described at <http://www.ibm.com/developerworks/java/library/j-jtp09263/>.
+ *
+ * <p>These attributes are just documentation. They don't have any run-time effect. The main aim is
+ * mainly just to standardize the terminology.
+ *
+ * <p>See ThreadSafetyTest for examples of how these attributes should be used.
  */
+// TODO(adonovan): prefer javax.annotation.concurrent.Immutable et al.
 public class ThreadSafety {
   /**
-   * The Immutable attribute indicates that instances of the class are
-   * immutable, or at least appear that way are far as their external API
-   * is concerned.  Immutable classes are usually also ThreadSafe,
-   * but can be ThreadHostile if they perform unsynchronized access to
-   * mutable static data.  (We deviate from Bloch's nomenclature by
-   * not assuming that Immutable implies ThreadSafe; developers should
-   * explicitly annotate classes as both Immutable and ThreadSafe when
+   * The Immutable attribute indicates that instances of the class are immutable, or at least appear
+   * that way are far as their external API is concerned. Immutable classes are usually also
+   * ThreadSafe, but can be ThreadHostile if they perform unsynchronized access to mutable static
+   * data. (We deviate from Bloch's nomenclature by not assuming that Immutable implies ThreadSafe;
+   * developers should explicitly annotate classes as both Immutable and ThreadSafe when
    * appropriate.)
    */
   @Documented
   @Target(value = {ElementType.TYPE})
-  @Retention(RetentionPolicy.RUNTIME)
+  @Retention(RetentionPolicy.SOURCE)
   public @interface Immutable {}
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
index ef48600..21cbb7a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java

@@ -102,6 +102,11 @@
     this.containingPackage = pkg;
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   public EnvironmentLabels getEnvironmentLabels() {
     environmentLabels.checkInitialized();
     return environmentLabels;

diff --git a/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java b/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java
index 51cdb1a..4221fb6 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java

@@ -16,6 +16,7 @@
 
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.License.DistributionType;
 import com.google.devtools.build.lib.util.FileType;
 import java.util.Set;
@@ -24,6 +25,7 @@
  * Common superclass for InputFile and OutputFile which provides implementation for the file
  * operations in common.
  */
+@Immutable
 public abstract class FileTarget implements Target, FileType.HasFileType {
   protected final Package pkg;
   protected final Label label;
@@ -37,6 +39,11 @@
     this.label = label;
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   public String getFilename() {
     return label.getName();
   }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/FilesetEntry.java b/src/main/java/com/google/devtools/build/lib/packages/FilesetEntry.java
index 5d2f66e..b937bf3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/FilesetEntry.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/FilesetEntry.java

@@ -47,10 +47,7 @@
 
   @Override
   public boolean isImmutable() {
-    // TODO(laszlocsomor): set this to true. I think we could do this right now, but am not sure.
-    // Maybe we have to verify that Starlark recognizes every member's type to be recursively
-    // immutable; as of 15/01/2016 this is not true for enum types in general, to name an example.
-    return false;
+    return true;
   }
 
   public static List<String> makeStringList(List<Label> labels) {

diff --git a/src/main/java/com/google/devtools/build/lib/packages/License.java b/src/main/java/com/google/devtools/build/lib/packages/License.java
index 6f77f8a..ad34370 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/License.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/License.java

@@ -244,6 +244,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // licences are Starlark-hashable
+  }
+
+  @Override
   public void repr(Printer printer) {
     printer.append(this.toString());
   }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java b/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
index af36bfc..d65e47d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java

@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.packages;
 
 import com.google.common.collect.ImmutableCollection;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.syntax.CallUtils;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Location;
@@ -21,8 +22,10 @@
 
 /**
  * Abstract base class for implementations of {@link StructImpl} that expose
- * StarlarkCallable-annotated fields (not just methods) to Starlark code.
+ * StarlarkCallable-annotated fields (not just methods) to Starlark code. Subclasses must be
+ * immutable.
  */
+@Immutable
 public abstract class NativeInfo extends StructImpl {
 
   protected NativeInfo(Provider provider) {
@@ -33,6 +36,11 @@
     super(provider, loc);
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   // TODO(adonovan): logically this should be a parameter of getValue
   // and getFieldNames or an instance field of this object.
   private static final StarlarkSemantics SEMANTICS = StarlarkSemantics.DEFAULT;

diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
index 5cf3f82..9dac3cf 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java

@@ -72,6 +72,11 @@
     return StarlarkProviderIdentifier.forKey(getKey());
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /**
    * equals() implements singleton class semantics.
    *

diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageGroupsRuleVisibility.java b/src/main/java/com/google/devtools/build/lib/packages/PackageGroupsRuleVisibility.java
index d110729..a90289e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageGroupsRuleVisibility.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageGroupsRuleVisibility.java

@@ -16,15 +16,12 @@
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
 import java.util.Collection;
 import java.util.List;
 
-/**
- * A rule visibility that allows visibility to a list of package groups.
- */
-@Immutable @ThreadSafe
+/** A rule visibility that allows visibility to a list of package groups. */
+@Immutable
 public class PackageGroupsRuleVisibility implements RuleVisibility {
   private final List<Label> packageGroups;
   private final PackageGroupContents directPackages;

diff --git a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
index 710d076..20aab02 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java

@@ -79,6 +79,11 @@
     this.configConditions = configConditions;
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   public ImmutableMap<Label, ConfigMatchingProvider> getConfigConditions() {
     return configConditions;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidStarlarkApiProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidStarlarkApiProvider.java
index 7b66231..c365950 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidStarlarkApiProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidStarlarkApiProvider.java

@@ -48,6 +48,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public Artifact getApk() {
     return getIdeInfoProvider().getSignedApk();
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java
index b3d441f..72fbe36 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java

@@ -421,6 +421,11 @@
     }
 
     @Override
+    public boolean isImmutable() {
+      return true; // immutable and Starlark-hashable
+    }
+
+    @Override
     public String toString() {
       return mode;
     }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/ApplePlatform.java b/src/main/java/com/google/devtools/build/lib/rules/apple/ApplePlatform.java
index 746c8fd..dfe6090 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/ApplePlatform.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/ApplePlatform.java

@@ -75,6 +75,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public PlatformType getType() {
     return platformType;
   }
@@ -222,6 +227,11 @@
     }
 
     @Override
+    public boolean isImmutable() {
+      return true; // immutable and Starlark-hashable
+    }
+
+    @Override
     public String toString() {
       return name().toLowerCase();
     }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java
index 92ec5c5..fe461f3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java

@@ -136,6 +136,11 @@
             (rule, attributes, appleConfig) -> appleConfig.getXcodeConfigLabel());
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /**
    * Returns the platform directory inside of Xcode for a given configuration.
    */

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
index d5d8844..e12c486 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java

@@ -233,6 +233,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public int compareTo(DottedVersion other) {
     int maxComponents = Math.max(components.size(), other.components.size());
     for (int componentIndex = 0; componentIndex < maxComponents; componentIndex++) {

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/swift/SwiftConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/swift/SwiftConfiguration.java
index e4cb491..0a9960b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/swift/SwiftConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/swift/SwiftConfiguration.java

@@ -36,6 +36,11 @@
     this.copts = ImmutableList.copyOf(options.copts);
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /** Returns a list of options to use for compiling Swift. */
   @Override
   public ImmutableList<String> getCopts() {

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
index b2c8cfe..03ca8e3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java

@@ -139,6 +139,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public Depset getStarlarkDefines() {
     return Depset.of(Depset.ElementType.STRING, getDefines());
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java
index f2d50ad..7ed4b1c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java

@@ -187,6 +187,11 @@
     }
 
     @Override
+    public boolean isImmutable() {
+      return true; // immutable and Starlark-hashable
+    }
+
+    @Override
     public Label getSkylarkOwner() throws EvalException {
       if (owner == null) {
         throw Starlark.errorf(

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java
index ff7a424..0161e42 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java

@@ -1330,6 +1330,11 @@
     }
 
     @Override
+    public boolean isImmutable() {
+      return true; // immutable and Starlark-hashable
+    }
+
+    @Override
     Map<String, Object> getVariablesMap() {
       return variablesMap;
     }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java
index 6e01b3c..d794957 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java

@@ -14,7 +14,6 @@
 // limitations under the License.
 
 import com.google.auto.value.AutoValue;
-import com.google.auto.value.AutoValue.CopyAnnotations;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
@@ -41,9 +40,13 @@
  */
 @AutoValue
 @Immutable
-@CopyAnnotations
 public abstract class LibraryToLink implements LibraryToLinkApi<Artifact> {
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   public static final Depset.ElementType TYPE = Depset.ElementType.of(LibraryToLink.class);
 
   public Artifact getDynamicLibraryForRuntimeOrNull(boolean linkingStatically) {

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCcLinkParamsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCcLinkParamsProvider.java
index 89d5c05..8d7dd89 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCcLinkParamsProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCcLinkParamsProvider.java

@@ -36,6 +36,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public Provider getProvider() {
     return PROVIDER;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationInfoProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationInfoProvider.java
index 0982b0b..b938f26 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationInfoProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationInfoProvider.java

@@ -37,6 +37,11 @@
   @Nullable private final NestedSet<Artifact> compilationClasspath;
   private final BootClassPathInfo bootClasspath;
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /** Builder for {@link JavaCompilationInfoProvider}. */
   public static class Builder {
     private ImmutableList<String> javacOpts;

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java
index 4063866..6c78f07 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java

@@ -90,6 +90,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public boolean usesAnnotationProcessing() {
     return usesAnnotationProcessing;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleOutputJarsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleOutputJarsProvider.java
index 1995870..29aa7d0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleOutputJarsProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleOutputJarsProvider.java

@@ -62,6 +62,11 @@
       this.srcJars = ImmutableList.copyOf(srcJars);
     }
 
+    @Override
+    public boolean isImmutable() {
+      return true; // immutable and Starlark-hashable
+    }
+
     @Nullable
     @Override
     public Artifact getClassJar() {
@@ -124,6 +129,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public ImmutableList<OutputJar> getOutputJars() {
     return outputJars;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeClasspathProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeClasspathProvider.java
index a504f77..481150b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeClasspathProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeClasspathProvider.java

@@ -38,6 +38,11 @@
     this.runtimeClasspath = runtimeClasspath;
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /** Returns the artifacts included on the runtime classpath of this binary. */
   @Override
   public Depset /*<Artifact>*/ getRuntimeClasspath() {

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeInfo.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeInfo.java
index ef9a677..74fae69 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeInfo.java

@@ -56,6 +56,11 @@
         javaBinaryRunfilesPath);
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   // Helper methods to access an instance of JavaRuntimeInfo.
 
   public static JavaRuntimeInfo forHost(RuleContext ruleContext) {

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
index 6f2052f..84d2a4a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java

@@ -677,6 +677,11 @@
   }
 
   @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
+  @Override
   public BuiltinProvider<ObjcProvider> getProvider() {
     return STARLARK_CONSTRUCTOR;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
index 55b1a92..9a67761 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java

@@ -75,6 +75,11 @@
     this.defaultToExplicitInitPy = defaultToExplicitInitPy;
   }
 
+  @Override
+  public boolean isImmutable() {
+    return true; // immutable and Starlark-hashable
+  }
+
   /**
    * Returns the Python version to use.
    *

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BUILD b/src/main/java/com/google/devtools/build/lib/syntax/BUILD
index 9e455af..9381bd1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BUILD

@@ -70,7 +70,6 @@
         "UnaryOperatorExpression.java",
     ],
     deps = [
-        "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/starlark/spelling",
@@ -125,7 +124,6 @@
     data = [":libcpu_profiler.so"],  # will be loaded dynamically
     deps = [
         ":frontend",
-        "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/skylarkinterface",

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index 776547d8..0e832d3 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java

@@ -16,7 +16,6 @@
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.collect.Ordering;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.starlark.spelling.SpellChecker;
 import java.util.IllegalFormatException;
 
@@ -112,54 +111,47 @@
   }
 
   /**
-   * Is this object known or assumed to be recursively hashable by Starlark?
-   *
-   * @param o an Object
-   * @return true if the object is known to be a hashable value.
+   * Reports whether a legal Starlark value is considered hashable to Starlark, and thus suitable as
+   * a key in a dict.
    */
-  // TODO(adonovan): obviate isHashable query by "try hash, catch StarlarkUnhashable",
-  // an unchecked exception. It is inefficient and potentially inconsistent to ask before doing.
   static boolean isHashable(Object o) {
+    // Bazel makes widespread assumptions that all Starlark values can be hashed
+    // by Java code, so we cannot implement isHashable by having
+    // StarlarkValue.hashCode throw an unchecked exception, which would be more
+    // efficient. Instead, before inserting a value in a dict, we must first ask
+    // it whether it isHashable, and then call its hashCode method only if so.
+    // For structs and tuples, this unfortunately visits the object graph twice.
+    //
+    // One subtlety: the struct.isHashable recursively asks whether its
+    // elements are immutable, not hashable. Consequently, even though a list
+    // may not be used as a dict key (even if frozen), a struct containing
+    // a list is hashable. TODO(adonovan): fix this inconsistency.
+    // Requires an incompatible change flag.
     if (o instanceof StarlarkValue) {
       return ((StarlarkValue) o).isHashable();
     }
-    return isImmutable(o.getClass());
+    return isImmutable(o);
   }
 
-  /**
-   * Is this object known or assumed to be recursively immutable by Starlark?
-   *
-   * @param o an Object
-   * @return true if the object is known to be an immutable value.
-   */
+  /** Reports whether a Starlark value is assumed to be deeply immutable. */
   // TODO(adonovan): eliminate the concept of querying for immutability. It is currently used for
   // only one purpose, the precondition for adding an element to a Depset, but Depsets should check
   // hashability, like Dicts. (Similarly, querying for hashability should go: just attempt to hash a
   // value, and be prepared for it to fail.) In practice, a value may be immutable, either
   // inherently (e.g. string) or because it has become frozen, but we don't need to query for it.
-  // Just attempt a mutation and be preared for it to fail.
+  // Just attempt a mutation and be prepared for it to fail.
   // It is inefficient and potentially inconsistent to ask before doing.
-  public static boolean isImmutable(Object o) {
-    if (o instanceof StarlarkValue) {
-      return ((StarlarkValue) o).isImmutable();
-    }
-    return isImmutable(o.getClass());
-  }
+  public static boolean isImmutable(Object x) {
+    // NB: This is used as the basis for accepting objects in Depsets,
+    // as well as for accepting objects as keys for Starlark dicts.
 
-  /**
-   * Is this class known to be *recursively* immutable by Starlark? For instance, class Tuple is not
-   * it, because it can contain mutable values.
-   *
-   * @param c a Class
-   * @return true if the class is known to represent only recursively immutable values.
-   */
-  // NB: This is used as the basis for accepting objects in Depset-s,
-  // as well as for accepting objects as keys for Starlark dict-s.
-  private static boolean isImmutable(Class<?> c) {
-    return c.isAnnotationPresent(Immutable.class) // TODO(bazel-team): beware of containers!
-        || c.equals(String.class)
-        || c.equals(Integer.class)
-        || c.equals(Boolean.class);
+    if (x instanceof String || x instanceof Integer || x instanceof Boolean) {
+      return true;
+    } else if (x instanceof StarlarkValue) {
+      return ((StarlarkValue) x).isImmutable();
+    } else {
+      throw new IllegalArgumentException("invalid Starlark value: " + x.getClass());
+    }
   }
 
   static void addIterator(Object x) {

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FileLocations.java b/src/main/java/com/google/devtools/build/lib/syntax/FileLocations.java
index 8dd5528..44da0a4 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FileLocations.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FileLocations.java

@@ -16,10 +16,10 @@
 
 import com.google.common.collect.Interner;
 import com.google.common.collect.Interners;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.util.Arrays;
 import java.util.Objects;
+import javax.annotation.concurrent.Immutable;
 
 /**
  * FileLocations maps each source offset within a file to a Location. An offset is a (UTF-16) char

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Location.java b/src/main/java/com/google/devtools/build/lib/syntax/Location.java
index b453f22..87da128 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Location.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Location.java

@@ -15,9 +15,9 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.Serializable;
+import javax.annotation.concurrent.Immutable;
 
 /**
  * A Location denotes a position within a Starlark file.

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/NoneType.java b/src/main/java/com/google/devtools/build/lib/syntax/NoneType.java
index e620287..ed99eef 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/NoneType.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/NoneType.java

@@ -14,8 +14,8 @@
 
 package com.google.devtools.build.lib.syntax;
 
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skylarkinterface.StarlarkBuiltin;
+import javax.annotation.concurrent.Immutable;
 
 /** The type of the Starlark None value. */
 @StarlarkBuiltin(

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/RangeList.java b/src/main/java/com/google/devtools/build/lib/syntax/RangeList.java
index d07d1ba..c8c9b9a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/RangeList.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/RangeList.java

@@ -16,12 +16,12 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.UnmodifiableIterator;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skylarkinterface.StarlarkBuiltin;
 import com.google.devtools.build.lib.skylarkinterface.StarlarkDocumentationCategory;
 import java.util.AbstractList;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
+import javax.annotation.concurrent.Immutable;
 
 /**
  * A sequence returned by the {@code range} function invocation.

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Starlark.java b/src/main/java/com/google/devtools/build/lib/syntax/Starlark.java
index 110e566..0b373fd 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Starlark.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Starlark.java

@@ -15,7 +15,6 @@
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skylarkinterface.StarlarkBuiltin;
 import com.google.devtools.build.lib.skylarkinterface.StarlarkInterfaceUtils;
 import com.google.errorprone.annotations.CheckReturnValue;
@@ -27,6 +26,7 @@
 import java.util.Map;
 import java.util.Set;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
 
 /**
  * The Starlark class defines the most important entry points, constants, and functions needed by
@@ -76,7 +76,7 @@
     env //
         .put("False", false)
         .put("True", true)
-        .put("None", Starlark.NONE);
+        .put("None", NONE);
     addMethods(env, new MethodLibrary());
     return env.build();
   }
@@ -114,7 +114,7 @@
   public static Object fromJava(Object x, @Nullable Mutability mutability) {
     if (x == null) {
       return NONE;
-    } else if (Starlark.valid(x)) {
+    } else if (valid(x)) {
       return x;
     } else if (x instanceof List) {
       return StarlarkList.copyOf(mutability, (List<?>) x);
@@ -152,11 +152,10 @@
    */
   public static void checkMutable(Mutability.Freezable x) throws EvalException {
     if (x.mutability().isFrozen()) {
-      throw Starlark.errorf("trying to mutate a frozen %s value", Starlark.type(x));
+      throw errorf("trying to mutate a frozen %s value", type(x));
     }
     if (x.updateIteratorCount(0)) {
-      throw Starlark.errorf(
-          "%s value is temporarily immutable due to active for-loop iteration", Starlark.type(x));
+      throw errorf("%s value is temporarily immutable due to active for-loop iteration", type(x));
     }
   }
 
@@ -492,7 +491,7 @@
   // tiny steps are headed. It doesn't work yet, but it helps to remember our direction.
   //
   // The API assumes that the "universe" portion (None, len, str) of the "predeclared" lexical block
-  // is always available, so clients needn't mention it in the API. Starlark.UNIVERSE will expose it
+  // is always available, so clients needn't mention it in the API. UNIVERSE will expose it
   // as a public constant.
   //
   // Q. is there any value to returning the Module as opposed to just its global bindings as a Map?

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkThread.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkThread.java
index 45f9914..89c5511 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkThread.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkThread.java

@@ -18,7 +18,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -30,6 +29,7 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Predicate;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
 
 /**
  * An StarlarkThread represents a Starlark thread.

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkValue.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkValue.java
index 3d2699b..8a14128 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkValue.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkValue.java

@@ -14,8 +14,6 @@
 
 package com.google.devtools.build.lib.syntax;
 
-import com.google.devtools.build.lib.concurrent.ThreadSafety;
-
 /** Base interface for all Starlark values besides boxed Java primitives. */
 public interface StarlarkValue {
 
@@ -64,27 +62,17 @@
     return true;
   }
 
-  /**
-   * Returns if the value is immutable.
-   *
-   * <p>Immutability is deep, i.e. in order for a value to be immutable, all values it is composed
-   * of must be immutable, too.
-   */
+  /** Reports whether the value is deeply immutable. */
   // TODO(adonovan): eliminate this concept. All uses really need to know is, is it hashable?,
   // because Starlark values must have stable hashes: a hashable value must either be immutable or
   // its hash must be part of its identity.
   // But this must wait until --incompatible_disallow_hashing_frozen_mutables=true is removed.
   // (see github.com/bazelbuild/bazel/issues/7800)
   default boolean isImmutable() {
-    // TODO(adonovan): this is an abuse of an unrelated annotation.
-    return getClass().isAnnotationPresent(ThreadSafety.Immutable.class);
+    return false;
   }
 
-  /**
-   * Returns if the value is hashable and thus suitable for being used as a dictionary key.
-   *
-   * <p>Hashability implies immutability, but not vice versa.
-   */
+  /** Reports whether the Starlark value is hashable and thus suitable as a dict key. */
   default boolean isHashable() {
     return this.isImmutable();
   }