diff options
Diffstat (limited to 'src/main/java/com/google/devtools')
4 files changed, 48 insertions, 48 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java index 6550ca2187..3e9ca1583c 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java @@ -99,6 +99,7 @@ public final class Environment implements Freezable { public static final class Frame implements Freezable { private final Mutability mutability; + @Nullable final Frame parent; final Map<String, Object> bindings = new LinkedHashMap<>(); // The label for the target this frame is defined in (e.g., //foo:bar.bzl). @@ -486,8 +487,8 @@ public final class Environment implements Freezable { @Nullable Label callerLabel) { this.globalFrame = Preconditions.checkNotNull(globalFrame); this.dynamicFrame = Preconditions.checkNotNull(dynamicFrame); - Preconditions.checkArgument(globalFrame.mutability().isMutable()); - Preconditions.checkArgument(dynamicFrame.mutability().isMutable()); + Preconditions.checkArgument(!globalFrame.mutability().isFrozen()); + Preconditions.checkArgument(!dynamicFrame.mutability().isFrozen()); this.semantics = semantics; this.eventHandler = eventHandler; this.importedExtensions = importedExtensions; @@ -563,9 +564,9 @@ public final class Environment implements Freezable { /** Builds the Environment. */ public Environment build() { - Preconditions.checkArgument(mutability.isMutable()); + Preconditions.checkArgument(!mutability.isFrozen()); if (parent != null) { - Preconditions.checkArgument(!parent.mutability().isMutable()); + Preconditions.checkArgument(parent.mutability().isFrozen()); } Frame globalFrame = new Frame(mutability, parent); Frame dynamicFrame = new Frame(mutability, null); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Mutability.java b/src/main/java/com/google/devtools/build/lib/syntax/Mutability.java index 78879afa21..6c0f523576 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Mutability.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Mutability.java @@ -23,46 +23,45 @@ import java.util.IdentityHashMap; import java.util.List; /** - * A Mutability is a resource associated with an {@link Environment} during an evaluation, - * that gives those who possess it a revokable capability to mutate this Environment and - * the objects created within that {@link Environment}. At the end of the evaluation, - * the resource is irreversibly closed, at which point the capability is revoked, - * and it is not possible to mutate either this {@link Environment} or these objects. + * Manages the capability to mutate an {@link Environment} and its contained Skylark objects. * - * <p>Evaluation in an {@link Environment} may thus mutate objects created in the same - * {@link Environment}, but may not mutate {@link Freezable} objects (lists, sets, dicts) - * created in a previous, concurrent or future {@link Environment}, and conversely, - * the bindings and objects in this {@link Environment} will be protected from - * mutation by evaluation in a different {@link Environment}. + * <p>Once the {@code Environment} is done evaluating, its {@code Mutability} is irreversibly closed + * ("frozen"). At that point, it is no longer possible to mutate either the {@code Environment} or + * its objects. This protects each {@code Environment} from unintentional and unsafe modification. + * Before freezing, only a single thread may use the {@code Environment}, but after freezing, any + * number of threads may access it. * - * <p>In addition, a Mutability tracks which of its objects are temporarily locked from mutation. - * This is used to prevent modification of iterables during loops. A Freezable may be locked - * multiple times (e.g., nested loops over the same iterable). Locking an object does not prohibit - * mutating its deeply contained values, e.g. in the case of a list of lists. + * <p>It is illegal for an evaluation in one {@code Environment} to affect another {@code + * Environment}, even if the second {@code Environment} has not yet been frozen. * - * <p>Only a single thread may use the {@link Environment} and objects created within it while the - * Mutability is still mutable as tested by {@link #isMutable}. Once the Mutability resource - * is closed, the {@link Environment} and its objects are immutable and can be simultaneously used - * by arbitrarily many threads. + * <p>A {@code Mutability} also tracks which {@link Freezable} objects in its {@code Environment} + * are temporarily locked from mutation. This is used to prevent modification of iterables during + * loops. A {@code Freezable} may be locked multiple times (e.g., nested loops over the same + * iterable). Locking an object does not prohibit mutating its deeply contained values, such as in + * the case of a list of lists. * - * <p>The safe usage of a Mutability requires to always use try-with-resource style: - * <code>try (Mutability mutability = Mutability.create(fmt, ...)) { ... }</code> - * Thus, you can create a Mutability, build an {@link Environment}, mutate that {@link Environment} - * and create mutable objects as you evaluate in that {@link Environment}, and finally return the - * resulting {@link Environment}, at which point the resource is closed, and the {@link Environment} - * and the objects it contains all become immutable. - * (Unsafe usage is allowed only in test code that is not part of the Bazel jar.) + * <p>To ensure safety, {@code Mutability}s must be created using the try-with-resource style: + * <pre>{@code + * try (Mutability mutability = Mutability.create(fmt, ...)) { ... } + * }</pre> + * The general pattern is to create a {@code Mutability}, build an {@code Environment}, mutate that + * {@code Environment} and its objects, and possibly return the result from within the {@code try} + * block, relying on the try-with-resource construct to ensure that everything gets frozen before + * the result is used. The only code that should create a {@code Mutability} without using + * try-with-resource is test code that is not part of the Bazel jar. */ // TODO(bazel-team): When we start using Java 8, this safe usage pattern can be enforced // through the use of a higher-order function. public final class Mutability implements AutoCloseable, Serializable { - private boolean isMutable; + private boolean isFrozen; + // For each locked Freezable, store all Locations where it is locked. // This field is set null once the Mutability is closed. This saves some // space, and avoids a concurrency bug from multiple Skylark modules // accessing the same Mutability at once. private IdentityHashMap<Freezable, List<Location>> lockedItems; + private final String annotation; // For error reporting. /** @@ -71,7 +70,7 @@ public final class Mutability implements AutoCloseable, Serializable { * describing to the user the context in which this Mutability was active. */ private Mutability(String annotation) { - this.isMutable = true; + this.isFrozen = false; // Seems unlikely that we'll often lock more than 10 things at once. this.lockedItems = new IdentityHashMap<>(10); this.annotation = Preconditions.checkNotNull(annotation); @@ -95,22 +94,22 @@ public final class Mutability implements AutoCloseable, Serializable { @Override public String toString() { - return String.format(isMutable ? "[%s]" : "(%s)", annotation); + return String.format(isFrozen ? "(%s)" : "[%s]", annotation); } - boolean isMutable() { - return isMutable; + boolean isFrozen() { + return isFrozen; } /** - * Return whether a {@link Freezable} belonging to this Mutability is currently locked. + * Return whether a {@link Freezable} belonging to this {@code Mutability} is currently locked. * Frozen objects are not considered locked, though they are of course immutable nonetheless. */ public boolean isLocked(Freezable object) { if (!object.mutability().equals(this)) { throw new AssertionError("trying to check the lock of an object from a different context"); } - if (!isMutable) { + if (isFrozen) { return false; } return lockedItems.containsKey(object); @@ -136,7 +135,7 @@ public final class Mutability implements AutoCloseable, Serializable { if (!object.mutability().equals(this)) { throw new AssertionError("trying to lock an object from a different context"); } - if (!isMutable) { + if (isFrozen) { return; } List<Location> locList; @@ -158,7 +157,7 @@ public final class Mutability implements AutoCloseable, Serializable { if (!object.mutability().equals(this)) { throw new AssertionError("trying to unlock an object from a different context"); } - if (!isMutable) { + if (isFrozen) { // It's okay if we somehow got frozen while there were still locked objects. return; } @@ -183,7 +182,7 @@ public final class Mutability implements AutoCloseable, Serializable { public void close() { // No need to track per-Freezable info since everything is immutable now. lockedItems = null; - isMutable = false; + isFrozen = true; } /** @@ -205,15 +204,15 @@ public final class Mutability implements AutoCloseable, Serializable { } /** - * Each {@link Freezable} object possesses a revokable Mutability attribute telling whether - * the object is still mutable. All {@link Freezable} objects created in the same - * {@link Environment} will share the same Mutability, inherited from this {@link Environment}. - * Only evaluation in the same {@link Environment} is allowed to mutate these objects, - * and only until the Mutability is irreversibly revoked. + * Each {@code Freezable} object possesses a {@link Mutability} that determines whether the object + * is still mutable. All {@code Freezable} objects created in the same {@link Environment} will + * share the same {@code Mutability}, inherited from this {@code Environment}. Only evaluation in + * the same {@code Environment} is allowed to mutate these objects, and only until the + * {@code Mutability} is irreversibly frozen. */ public interface Freezable { /** - * Returns the {@link Mutability} capability associated with this Freezable object. + * Returns the {@link Mutability} associated with this Freezable object. * This should not change over the lifetime of the object. */ Mutability mutability(); @@ -228,7 +227,7 @@ public final class Mutability implements AutoCloseable, Serializable { */ public static void checkMutable(Freezable object, Environment env) throws MutabilityException { - if (!object.mutability().isMutable()) { + if (object.mutability().isFrozen()) { // Throw MutabilityException, not AssertionError, even if the object was from // another context. throw new MutabilityException("trying to mutate a frozen object"); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkMutable.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkMutable.java index a88017318a..941b7f3a93 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkMutable.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkMutable.java @@ -47,7 +47,7 @@ public abstract class SkylarkMutable implements Freezable, SkylarkValue { @Override public boolean isImmutable() { - return !mutability().isMutable(); + return mutability().isFrozen(); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java index 4b81e227c3..3712edc5ec 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java @@ -51,7 +51,7 @@ public class UserDefinedFunction extends BaseFunction { @Override public Object call(Object[] arguments, FuncallExpression ast, Environment env) throws EvalException, InterruptedException { - if (!env.mutability().isMutable()) { + if (env.mutability().isFrozen()) { throw new EvalException(getLocation(), "Trying to call in frozen environment"); } if (env.getStackTrace().contains(this)) { |