Rename fields to make them harder to use by accident.

Nearly all access should be performed through the various methods (e.g., `casListeners(...)`, `listeners()`). In fact, we could be principled and perform _all_ access through those methods (except of course for within the implementations of those methods themselves). I haven't done that here just out of fear that it will somehow affect performance or cause stack overflows.

Accidental usage has been something that I've historically worried about in, e.g., `AbstractFuture.set(V value)`, whose parameter has had the same name as a field. That _particular_ example matters less at the moment because the field recently became `private` to a new superclass, `AbstractFutureState`, and so it's not accessible in the subclass `AbstractFuture`.

But it's going to matter again: I'm likely to make the fields package-private as part of [work to migrate `guava-android` off `Unsafe`](#7742). Currently, the fields can be `private` because we call `MethodHandles.lookup()` from within `AbsractFutureState`. (Yes, it would initially seem that [we shouldn't have to](https://github.com/google/guava/blob/c7363f7fb40698bb5f99d198cc45884f38642f86/guava/src/com/google/common/util/concurrent/AbstractFutureState.java#L612-L632), but we do.) But that requires `AbstractFutureState` to refer to `MethodHandles.Lookup` in one of its method signatures\[*\], and that makes old versions of Android upset. To avoid that, I will make `value` package-private, at which point I won't need the `MethodHandles.Lookup` reference in `AbstractFutureState` itself.

And when I tried making `value` package-private, _one test_ started to fail. It should have taken me less time to figure out, but I eventually discovered that the problem was that [the test refers to "`value`" inside an `AbstractFuture` subclass](https://github.com/google/guava/blob/c7363f7fb40698bb5f99d198cc45884f38642f86/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java#L78). Previously, this referred to the local variable `value` from the test method; with my change, it was instead referring to the `value` field.

(I wouldn't have to have gone down the road of making the field non-`private` in the first place [if not for Java 8 compatibility](#6614 (comment)).... Still, as discussed above, this rename could protect against problems _within_ the file, too, and such problems could arise even if the field were to remain `private`.)

\[*\] Or maybe I could declare the method as returning `Object` instead of `MethodHandles.Lookup`, and the caller could cast it back? But we found during [our `SequencedCollection` work](#6903) that Android (and I think maybe the JVM, but I can't find my record of this) can produce a `VerifyError` in some cases in which _implementation_ code refers to an unknown type, I think specifically when it needs to check whether a `return someThingOfTypeFoo` from that method is compatible with the return type `Bar` of the method. We _might_ be able to work around that by performing an explicit, "redundant" cast to `Object`, but I'm not even sure how to get javac to do that, and it feels very fragile, especially in the presence of optimization/minification tools.

RELNOTES=n/a
PiperOrigin-RevId: 741511657

Author: cpovirk

android/guava/src/com/google/common/util/concurrent/AbstractFutureState.java

@@ -49,39 +49,39 @@ abstract class AbstractFutureState<V extends @Nullable Object> extends InternalF
     implements ListenableFuture<V> {
   /**
    * Performs a {@linkplain java.lang.invoke.VarHandle#compareAndSet compare-and-set} operation on
-   * the {@link #listeners} field.
+   * {@link #listenersField}.
    */
   final boolean casListeners(@Nullable Listener expect, Listener update) {
     return ATOMIC_HELPER.casListeners(this, expect, update);
   }
 
   /**
-   * Performs a {@linkplain java.lang.invoke.VarHandle#getAndSet get-and-set} operation on the
-   * {@link #listeners} field..
+   * Performs a {@linkplain java.lang.invoke.VarHandle#getAndSet get-and-set} operation on {@link
+   * #listenersField}.
    */
   final @Nullable Listener gasListeners(Listener update) {
     return ATOMIC_HELPER.gasListeners(this, update);
   }
 
   /**
    * Performs a {@linkplain java.lang.invoke.VarHandle#compareAndSet compare-and-set} operation on
-   * the {@link #value} field of {@code future}.
+   * {@link #valueField} of {@code future}.
    */
   static boolean casValue(AbstractFutureState<?> future, @Nullable Object expect, Object update) {
     return ATOMIC_HELPER.casValue(future, expect, update);
   }
 
   /** Returns the value of the future, using a volatile read. */
   final @Nullable Object value() {
-    return value;
+    return valueField;
   }
 
   /** Returns the head of the listener stack, using a volatile read. */
   final @Nullable Listener listeners() {
-    return listeners;
+    return listenersField;
   }
 
-  /** Releases all threads in the {@link #waiters} list, and clears the list. */
+  /** Releases all threads in the {@link #waitersField} list, and clears the list. */
   final void releaseWaiters() {
     Waiter head = gasWaiters(Waiter.TOMBSTONE);
     for (Waiter currentWaiter = head; currentWaiter != null; currentWaiter = currentWaiter.next) {
@@ -92,10 +92,10 @@ final void releaseWaiters() {
   // Gets and Timed Gets
   //
   // * Be responsive to interruption
-  // * Don't create Waiter nodes if you aren't going to park, this helps reduce contention on the
-  //   waiters field.
-  // * Future completion is defined by when #value becomes non-null/non DelegatingToFuture
-  // * Future completion can be observed if the waiters field contains a TOMBSTONE
+  // * Don't create Waiter nodes if you aren't going to park, this helps reduce contention on
+  //   waitersField.
+  // * Future completion is defined by when #valueField becomes non-null/non DelegatingToFuture
+  // * Future completion can be observed if the waitersField field contains a TOMBSTONE
 
   // Timed Get
   // There are a few design constraints to consider
@@ -127,15 +127,15 @@ final V blockingGet(long timeout, TimeUnit unit)
     if (Thread.interrupted()) {
       throw new InterruptedException();
     }
-    @RetainedLocalRef Object localValue = value;
+    @RetainedLocalRef Object localValue = valueField;
     if (localValue != null & notInstanceOfDelegatingToFuture(localValue)) {
       return getDoneValue(localValue);
     }
     // we delay calling nanoTime until we know we will need to either park or spin
     long endNanos = remainingNanos > 0 ? System.nanoTime() + remainingNanos : 0;
     long_wait_loop:
     if (remainingNanos >= SPIN_THRESHOLD_NANOS) {
-      Waiter oldHead = waiters;
+      Waiter oldHead = waitersField;
       if (oldHead != Waiter.TOMBSTONE) {
         Waiter node = new Waiter();
         do {
@@ -151,7 +151,7 @@ final V blockingGet(long timeout, TimeUnit unit)
 
               // Otherwise re-read and check doneness. If we loop then it must have been a spurious
               // wakeup
-              localValue = value;
+              localValue = valueField;
               if (localValue != null & notInstanceOfDelegatingToFuture(localValue)) {
                 return getDoneValue(localValue);
               }
@@ -165,18 +165,18 @@ final V blockingGet(long timeout, TimeUnit unit)
               }
             }
           }
-          oldHead = waiters; // re-read and loop.
+          oldHead = waitersField; // re-read and loop.
         } while (oldHead != Waiter.TOMBSTONE);
       }
-      // re-read value, if we get here then we must have observed a TOMBSTONE while trying to add a
-      // waiter.
-      // requireNonNull is safe because value is always set before TOMBSTONE.
-      return getDoneValue(requireNonNull(value));
+      // re-read valueField, if we get here then we must have observed a TOMBSTONE while trying to
+      // add a waiter.
+      // requireNonNull is safe because valueField is always set before TOMBSTONE.
+      return getDoneValue(requireNonNull(valueField));
     }
     // If we get here then we have remainingNanos < SPIN_THRESHOLD_NANOS and there is no node on the
     // waiters list
     while (remainingNanos > 0) {
-      localValue = value;
+      localValue = valueField;
       if (localValue != null & notInstanceOfDelegatingToFuture(localValue)) {
         return getDoneValue(localValue);
       }
@@ -226,11 +226,11 @@ final V blockingGet() throws InterruptedException, ExecutionException {
     if (Thread.interrupted()) {
       throw new InterruptedException();
     }
-    @RetainedLocalRef Object localValue = value;
+    @RetainedLocalRef Object localValue = valueField;
     if (localValue != null & notInstanceOfDelegatingToFuture(localValue)) {
       return getDoneValue(localValue);
     }
-    Waiter oldHead = waiters;
+    Waiter oldHead = waitersField;
     if (oldHead != Waiter.TOMBSTONE) {
       Waiter node = new Waiter();
       do {
@@ -246,19 +246,19 @@ final V blockingGet() throws InterruptedException, ExecutionException {
             }
             // Otherwise re-read and check doneness. If we loop then it must have been a spurious
             // wakeup
-            localValue = value;
+            localValue = valueField;
             if (localValue != null & notInstanceOfDelegatingToFuture(localValue)) {
               return getDoneValue(localValue);
             }
           }
         }
-        oldHead = waiters; // re-read and loop.
+        oldHead = waitersField; // re-read and loop.
       } while (oldHead != Waiter.TOMBSTONE);
     }
-    // re-read value, if we get here then we must have observed a TOMBSTONE while trying to add a
-    // waiter.
-    // requireNonNull is safe because value is always set before TOMBSTONE.
-    return getDoneValue(requireNonNull(value));
+    // re-read valueField, if we get here then we must have observed a TOMBSTONE while trying to add
+    // a waiter.
+    // requireNonNull is safe because valueField is always set before TOMBSTONE.
+    return getDoneValue(requireNonNull(valueField));
   }
 
   /** Constructor for use by {@link AbstractFuture}. */
@@ -296,7 +296,7 @@ final V blockingGet() throws InterruptedException, ExecutionException {
     GENERATE_CANCELLATION_CAUSES = generateCancellationCauses;
   }
 
-  /** Waiter links form a Treiber stack, in the {@link #waiters} field. */
+  /** Waiter links form a Treiber stack in {@link #waitersField}. */
   static final class Waiter {
     static final Waiter TOMBSTONE = new Waiter(false /* ignored param */);
 
@@ -310,12 +310,12 @@ static final class Waiter {
     Waiter(boolean unused) {}
 
     Waiter() {
-      // avoid volatile write, write is made visible by subsequent CAS on waiters field
+      // avoid volatile write, write is made visible by subsequent CAS on waitersField field
       putThread(this, Thread.currentThread());
     }
 
-    // non-volatile write to the next field. Should be made visible by subsequent CAS on waiters
-    // field.
+    // non-volatile write to the next field. Should be made visible by a subsequent CAS on
+    // waitersField.
     void setNext(@Nullable Waiter next) {
       putNext(this, next);
     }
@@ -380,8 +380,8 @@ void unpark() {
     }
   }
 
-  // TODO(lukes): investigate using the @Contended annotation on these fields when jdk8 is
-  // available.
+  // TODO(lukes): Investigate using a @Contended annotation on these fields once one is available.
+
   /**
    * This field encodes the current state of the future.
    *
@@ -397,13 +397,13 @@ void unpark() {
    *       argument.
    * </ul>
    */
-  private volatile @Nullable Object value;
+  private volatile @Nullable Object valueField;
 
   /** All listeners. */
-  private volatile @Nullable Listener listeners;
+  private volatile @Nullable Listener listenersField;
 
   /** All waiting threads. */
-  private volatile @Nullable Waiter waiters;
+  private volatile @Nullable Waiter waitersField;
 
   /** Non-volatile write of the thread to the {@link Waiter#thread} field. */
   private static void putThread(Waiter waiter, Thread newValue) {
@@ -416,16 +416,16 @@ private static void putNext(Waiter waiter, @Nullable Waiter newValue) {
   }
 
   /**
-   * Performs a {@linkplain java.lang.invoke.VarHandle#compareAndSet compare-and-set} operation on
-   * the {@link #waiters} field.
+   * Performs a {@linkplain java.lang.invoke.VarHandle#compareAndSet compare-and-set} operation
+   * {@link #waitersField}.
    */
   private boolean casWaiters(@Nullable Waiter expect, @Nullable Waiter update) {
     return ATOMIC_HELPER.casWaiters(this, expect, update);
   }
 
   /**
-   * Performs a {@linkplain java.lang.invoke.VarHandle#getAndSet get-and-set} operation on the
-   * {@link #waiters} field.
+   * Performs a {@linkplain java.lang.invoke.VarHandle#getAndSet get-and-set} operation on {@link
+   * #waitersField}.
    */
   private final @Nullable Waiter gasWaiters(Waiter update) {
     return ATOMIC_HELPER.gasWaiters(this, update);
@@ -447,7 +447,7 @@ private void removeWaiter(Waiter node) {
     restart:
     while (true) {
       Waiter pred = null;
-      Waiter curr = waiters;
+      Waiter curr = waitersField;
       if (curr == Waiter.TOMBSTONE) {
         return; // give up if someone is calling complete
       }
@@ -481,21 +481,21 @@ private abstract static class AtomicHelper {
     /** Non-volatile write of the waiter to the {@link Waiter#next} field. */
     abstract void putNext(Waiter waiter, @Nullable Waiter newValue);
 
-    /** Performs a CAS operation on the {@link AbstractFutureState#waiters} field. */
+    /** Performs a CAS operation on {@link AbstractFutureState#waitersField}. */
     abstract boolean casWaiters(
         AbstractFutureState<?> future, @Nullable Waiter expect, @Nullable Waiter update);
 
-    /** Performs a CAS operation on the {@link AbstractFutureState#listeners} field. */
+    /** Performs a CAS operation on {@link AbstractFutureState#listenersField}. */
     abstract boolean casListeners(
         AbstractFutureState<?> future, @Nullable Listener expect, Listener update);
 
-    /** Performs a GAS operation on the {@link AbstractFutureState#waiters} field. */
+    /** Performs a GAS operation on {@link AbstractFutureState#waitersField}. */
     abstract @Nullable Waiter gasWaiters(AbstractFutureState<?> future, Waiter update);
 
-    /** Performs a GAS operation on the {@link AbstractFutureState#listeners} field. */
+    /** Performs a GAS operation on {@link AbstractFutureState#listenersField}. */
     abstract @Nullable Listener gasListeners(AbstractFutureState<?> future, Listener update);
 
-    /** Performs a CAS operation on the {@link AbstractFutureState#value} field. */
+    /** Performs a CAS operation on {@link AbstractFutureState#valueField}. */
     abstract boolean casValue(
         AbstractFutureState<?> future, @Nullable Object expect, Object update);
   }
@@ -541,10 +541,11 @@ private static final class UnsafeAtomicHelper extends AtomicHelper {
       }
       try {
         Class<?> abstractFutureState = AbstractFutureState.class;
-        WAITERS_OFFSET = unsafe.objectFieldOffset(abstractFutureState.getDeclaredField("waiters"));
+        WAITERS_OFFSET =
+            unsafe.objectFieldOffset(abstractFutureState.getDeclaredField("waitersField"));
         LISTENERS_OFFSET =
-            unsafe.objectFieldOffset(abstractFutureState.getDeclaredField("listeners"));
-        VALUE_OFFSET = unsafe.objectFieldOffset(abstractFutureState.getDeclaredField("value"));
+            unsafe.objectFieldOffset(abstractFutureState.getDeclaredField("listenersField"));
+        VALUE_OFFSET = unsafe.objectFieldOffset(abstractFutureState.getDeclaredField("valueField"));
         WAITER_THREAD_OFFSET = unsafe.objectFieldOffset(Waiter.class.getDeclaredField("thread"));
         WAITER_NEXT_OFFSET = unsafe.objectFieldOffset(Waiter.class.getDeclaredField("next"));
         UNSAFE = unsafe;
@@ -578,7 +579,7 @@ boolean casListeners(
     @Override
     @Nullable Listener gasListeners(AbstractFutureState<?> future, Listener update) {
       while (true) {
-        Listener listener = future.listeners;
+        Listener listener = future.listenersField;
         if (update == listener) {
           return listener;
         }
@@ -591,7 +592,7 @@ boolean casListeners(
     @Override
     @Nullable Waiter gasWaiters(AbstractFutureState<?> future, Waiter update) {
       while (true) {
-        Waiter waiter = future.waiters;
+        Waiter waiter = future.waitersField;
         if (update == waiter) {
           return waiter;
         }
@@ -717,8 +718,8 @@ void putNext(Waiter waiter, @Nullable Waiter newValue) {
     boolean casWaiters(
         AbstractFutureState<?> future, @Nullable Waiter expect, @Nullable Waiter update) {
       synchronized (future) {
-        if (future.waiters == expect) {
-          future.waiters = update;
+        if (future.waitersField == expect) {
+          future.waitersField = update;
           return true;
         }
         return false;
@@ -729,8 +730,8 @@ boolean casWaiters(
     boolean casListeners(
         AbstractFutureState<?> future, @Nullable Listener expect, Listener update) {
       synchronized (future) {
-        if (future.listeners == expect) {
-          future.listeners = update;
+        if (future.listenersField == expect) {
+          future.listenersField = update;
           return true;
         }
         return false;
@@ -740,9 +741,9 @@ boolean casListeners(
     @Override
     @Nullable Listener gasListeners(AbstractFutureState<?> future, Listener update) {
       synchronized (future) {
-        Listener old = future.listeners;
+        Listener old = future.listenersField;
         if (old != update) {
-          future.listeners = update;
+          future.listenersField = update;
         }
         return old;
       }
@@ -751,9 +752,9 @@ boolean casListeners(
     @Override
     @Nullable Waiter gasWaiters(AbstractFutureState<?> future, Waiter update) {
       synchronized (future) {
-        Waiter old = future.waiters;
+        Waiter old = future.waitersField;
         if (old != update) {
-          future.waiters = update;
+          future.waitersField = update;
         }
         return old;
       }
@@ -762,8 +763,8 @@ boolean casListeners(
     @Override
     boolean casValue(AbstractFutureState<?> future, @Nullable Object expect, Object update) {
       synchronized (future) {
-        if (future.value == expect) {
-          future.value = update;
+        if (future.valueField == expect) {
+          future.valueField = update;
           return true;
         }
         return false;