commit 121224e73780aa6d97988780c2bd1a2fa8a87e6b
author adonovan <adonovan@google.com> Mon May 18 08:39:45 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Mon May 18 08:41:16 2020 -0700
tree eb097ffcc5fcb16b6850d05285334243ee32dcda
parent 9ef27943fabcffd9038e19fbb4b79793cf25cc73

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