Skip to content

Commit 8f6f4b3

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Assorted cleanups noticed after recent AbstractFuture [changes](#2934).
RELNOTES=n/a PiperOrigin-RevId: 729532679
1 parent b15c23f commit 8f6f4b3

File tree

4 files changed

+26
-24
lines changed

4 files changed

+26
-24
lines changed

android/guava-tests/test/com/google/common/util/concurrent/AggregateFutureStateFallbackAtomicHelperTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,9 @@ private void runTestMethod(ClassLoader classLoader) throws Exception {
112112
private void checkHelperVersion(ClassLoader classLoader, String expectedHelperClassName)
113113
throws Exception {
114114
// Make sure we are actually running with the expected helper implementation
115-
Class<?> abstractFutureStateClass = classLoader.loadClass(AggregateFutureState.class.getName());
116-
Field helperField = abstractFutureStateClass.getDeclaredField("ATOMIC_HELPER");
115+
Class<?> aggregateFutureStateClass =
116+
classLoader.loadClass(AggregateFutureState.class.getName());
117+
Field helperField = aggregateFutureStateClass.getDeclaredField("ATOMIC_HELPER");
117118
helperField.setAccessible(true);
118119
assertEquals(expectedHelperClassName, helperField.get(null).getClass().getSimpleName());
119120
}

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@
6565
@GwtCompatible
6666
/*
6767
* TODO(cpovirk): Do we still need @ReflectionSupport on *this* class now that the fields live in
68-
* the superclass? Note that Listener (which we also reflect on) still leaves here.
68+
* the superclass? Note that Listener (which we also reflect on) still lives here.
6969
*/
7070
@ReflectionSupport(value = ReflectionSupport.Level.FULL)
7171
public abstract class AbstractFuture<V extends @Nullable Object> extends AbstractFutureState<V> {
@@ -126,7 +126,7 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
126126
}
127127
}
128128

129-
/** Listeners also form a stack through the {@link #listeners} field. */
129+
/** Listeners form a Treiber stack through the {@link #listeners} field. */
130130
static final class Listener {
131131
static final Listener TOMBSTONE = new Listener();
132132
// null only for TOMBSTONE
@@ -362,21 +362,21 @@ mayInterruptIfRunning, new CancellationException("Future.cancel() was called."))
362362
// care if we are successful or not.
363363
ListenableFuture<?> futureToPropagateTo = ((DelegatingToFuture) localValue).future;
364364
if (futureToPropagateTo instanceof Trusted) {
365-
// If the future is a TrustedFuture then we specifically avoid calling cancel()
365+
// If the future is a Trusted instance then we specifically avoid calling cancel()
366366
// this has 2 benefits
367367
// 1. for long chains of futures strung together with setFuture we consume less stack
368368
// 2. we avoid allocating Cancellation objects at every level of the cancellation
369369
// chain
370-
// We can only do this for TrustedFuture, because TrustedFuture.cancel is final and
371-
// does nothing but delegate to this method.
370+
// We can only do this for Trusted, because Trusted implementations of cancel do
371+
// nothing but delegate to this method and do not permit user overrides.
372372
AbstractFuture<?> trusted = (AbstractFuture<?>) futureToPropagateTo;
373373
localValue = trusted.value();
374374
if (localValue == null | localValue instanceof DelegatingToFuture) {
375375
abstractFuture = trusted;
376376
continue; // loop back up and try to complete the new future
377377
}
378378
} else {
379-
// not a TrustedFuture, call cancel directly.
379+
// not a Trusted instance, call cancel directly.
380380
futureToPropagateTo.cancel(mayInterruptIfRunning);
381381
}
382382
}
@@ -597,10 +597,10 @@ protected boolean setFuture(ListenableFuture<? extends V> future) {
597597
*/
598598
private static Object getFutureValue(ListenableFuture<?> future) {
599599
if (future instanceof Trusted) {
600-
// Break encapsulation for TrustedFuture instances since we know that subclasses cannot
601-
// override .get() (since it is final) and therefore this is equivalent to calling .get()
602-
// and unpacking the exceptions like we do below (just much faster because it is a single
603-
// field read instead of a read, several branches and possibly creating exceptions).
600+
// Break encapsulation for Trusted instances since we know that subclasses cannot override
601+
// .get() and therefore this is equivalent to calling .get() and unpacking the exceptions like
602+
// we do below (just much faster because it is a single field read instead of a read, several
603+
// branches and possibly creating exceptions).
604604
Object v = ((AbstractFuture<?>) future).value();
605605
if (v instanceof Cancellation) {
606606
// If the other future was interrupted, clear the interrupted bit while preserving the cause

guava-tests/test/com/google/common/util/concurrent/AggregateFutureStateFallbackAtomicHelperTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,9 @@ private void runTestMethod(ClassLoader classLoader) throws Exception {
112112
private void checkHelperVersion(ClassLoader classLoader, String expectedHelperClassName)
113113
throws Exception {
114114
// Make sure we are actually running with the expected helper implementation
115-
Class<?> abstractFutureStateClass = classLoader.loadClass(AggregateFutureState.class.getName());
116-
Field helperField = abstractFutureStateClass.getDeclaredField("ATOMIC_HELPER");
115+
Class<?> aggregateFutureStateClass =
116+
classLoader.loadClass(AggregateFutureState.class.getName());
117+
Field helperField = aggregateFutureStateClass.getDeclaredField("ATOMIC_HELPER");
117118
helperField.setAccessible(true);
118119
assertEquals(expectedHelperClassName, helperField.get(null).getClass().getSimpleName());
119120
}

guava/src/com/google/common/util/concurrent/AbstractFuture.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@
6565
@GwtCompatible
6666
/*
6767
* TODO(cpovirk): Do we still need @ReflectionSupport on *this* class now that the fields live in
68-
* the superclass? Note that Listener (which we also reflect on) still leaves here.
68+
* the superclass? Note that Listener (which we also reflect on) still lives here.
6969
*/
7070
@ReflectionSupport(value = ReflectionSupport.Level.FULL)
7171
public abstract class AbstractFuture<V extends @Nullable Object> extends AbstractFutureState<V> {
@@ -126,7 +126,7 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
126126
}
127127
}
128128

129-
/** Listeners also form a stack through the {@link #listeners} field. */
129+
/** Listeners form a Treiber stack through the {@link #listeners} field. */
130130
static final class Listener {
131131
static final Listener TOMBSTONE = new Listener();
132132
// null only for TOMBSTONE
@@ -362,21 +362,21 @@ mayInterruptIfRunning, new CancellationException("Future.cancel() was called."))
362362
// care if we are successful or not.
363363
ListenableFuture<?> futureToPropagateTo = ((DelegatingToFuture) localValue).future;
364364
if (futureToPropagateTo instanceof Trusted) {
365-
// If the future is a TrustedFuture then we specifically avoid calling cancel()
365+
// If the future is a Trusted instance then we specifically avoid calling cancel()
366366
// this has 2 benefits
367367
// 1. for long chains of futures strung together with setFuture we consume less stack
368368
// 2. we avoid allocating Cancellation objects at every level of the cancellation
369369
// chain
370-
// We can only do this for TrustedFuture, because TrustedFuture.cancel is final and
371-
// does nothing but delegate to this method.
370+
// We can only do this for Trusted, because Trusted implementations of cancel do
371+
// nothing but delegate to this method and do not permit user overrides.
372372
AbstractFuture<?> trusted = (AbstractFuture<?>) futureToPropagateTo;
373373
localValue = trusted.value();
374374
if (localValue == null | localValue instanceof DelegatingToFuture) {
375375
abstractFuture = trusted;
376376
continue; // loop back up and try to complete the new future
377377
}
378378
} else {
379-
// not a TrustedFuture, call cancel directly.
379+
// not a Trusted instance, call cancel directly.
380380
futureToPropagateTo.cancel(mayInterruptIfRunning);
381381
}
382382
}
@@ -597,10 +597,10 @@ protected boolean setFuture(ListenableFuture<? extends V> future) {
597597
*/
598598
private static Object getFutureValue(ListenableFuture<?> future) {
599599
if (future instanceof Trusted) {
600-
// Break encapsulation for TrustedFuture instances since we know that subclasses cannot
601-
// override .get() (since it is final) and therefore this is equivalent to calling .get()
602-
// and unpacking the exceptions like we do below (just much faster because it is a single
603-
// field read instead of a read, several branches and possibly creating exceptions).
600+
// Break encapsulation for Trusted instances since we know that subclasses cannot override
601+
// .get() and therefore this is equivalent to calling .get() and unpacking the exceptions like
602+
// we do below (just much faster because it is a single field read instead of a read, several
603+
// branches and possibly creating exceptions).
604604
Object v = ((AbstractFuture<?>) future).value();
605605
if (v instanceof Cancellation) {
606606
// If the other future was interrupted, clear the interrupted bit while preserving the cause

0 commit comments

Comments
 (0)