Skip to content

Cache the hashCode for dataflow expressions #1643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wmdietl merged 1 commit into from
Apr 21, 2026
+74 −18
Merged

Cache the hashCode for dataflow expressions #1643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion dataflow/src/main/java/org/checkerframework/dataflow/expression/ArrayAccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,15 @@ public boolean equals(@Nullable Object obj) {
return array.equals(other.array) && index.equals(other.index);
}

/** Cache the hashCode. Recomputed if zero. */
private int hashCodeCache = 0;

@Override
public int hashCode() {
return Objects.hash(array, index);
if (hashCodeCache == 0) {
hashCodeCache = Objects.hash(array, index);
}
return hashCodeCache;
Comment on lines +107 to +115
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hashCodeCache uses 0 as the "not computed" sentinel. If Objects.hash(array, index) computes to 0, this will recompute every call (and re-allocate) rather than caching. Use a separate boolean/nullable cache to distinguish "not computed" from a computed 0.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@wmdietl wmdietl Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would require yet another field to store "computed" value. Seems okay to re-compute if the hash happens to be 0 - all other cases are covered.

}

@Override
Expand Down
8 changes: 7 additions & 1 deletion dataflow/src/main/java/org/checkerframework/dataflow/expression/ArrayCreation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,15 @@ public boolean isModifiableByOtherCode() {
return true;
}

/** Cache the hashCode. Recomputed if zero. */
private int hashCodeCache = 0;

@Override
public int hashCode() {
return Objects.hash(dimensions, initializers, getType().toString());
if (hashCodeCache == 0) {
hashCodeCache = Objects.hash(dimensions, initializers, getType().toString());
}
return hashCodeCache;
Comment on lines +91 to +99
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This caches the hash code, but dimensions/initializers are stored and returned as mutable lists. If a caller mutates either list after hashCode() is first computed, the cached value becomes stale and breaks hash-based collections. Consider defensive copies (and returning unmodifiable views) to make these fields effectively immutable, or avoid caching here if mutation is expected. Also, using 0 as the cache sentinel means a computed 0 won’t be cached.

Copilot uses AI. Check for mistakes.
}

@Override
Expand Down
8 changes: 7 additions & 1 deletion dataflow/src/main/java/org/checkerframework/dataflow/expression/BinaryOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,15 @@ public boolean containsModifiableAliasOf(Store<?> store, JavaExpression other) {
|| right.containsModifiableAliasOf(store, other);
}

/** Cache the hashCode. Recomputed if zero. */
private int hashCodeCache = 0;

@Override
public int hashCode() {
return Objects.hash(operationKind, left, right);
if (hashCodeCache == 0) {
hashCodeCache = Objects.hash(operationKind, left, right);
}
return hashCodeCache;
}

Comment on lines +139 to 143
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

equals treats some operations as commutative (it considers (left,right) equal to (right,left)), but hashCode() is order-dependent (Objects.hash(operationKind, left, right)). This violates the equals/hashCode contract and can break use as a map/set key. Make hashCode() commutative for commutative operators (e.g., combine operand hashes in an order-independent way), or make equals order-sensitive.

Suggested change
hashCodeCache = Objects.hash(operationKind, left, right);
}
return hashCodeCache;
}
hashCodeCache = computeHashCode();
}
return hashCodeCache;
}
/**
* Computes a hash code consistent with equals(Object).
*
* <p>For commutative operations, the operand contribution must be order-independent because
* equals(Object) considers (left, right) equal to (right, left). For non-commutative
* operations, preserve the ordered hash.
*/
private int computeHashCode() {
if (isCommutative()) {
int leftHash = left.hashCode();
int rightHash = right.hashCode();
int first = Math.min(leftHash, rightHash);
int second = Math.max(leftHash, rightHash);
return Objects.hash(operationKind, first, second);
}
return Objects.hash(operationKind, left, right);
}

Copilot uses AI. Check for mistakes.
@Override
Expand Down
8 changes: 7 additions & 1 deletion dataflow/src/main/java/org/checkerframework/dataflow/expression/FieldAccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,15 @@ public boolean equals(@Nullable Object obj) {
|| this.getReceiver() instanceof ThisReference);
}

/** Cache the hashCode. Recomputed if zero. */
private int hashCodeCache = 0;

@Override
public int hashCode() {
return Objects.hash(getField(), getReceiver());
if (hashCodeCache == 0) {
hashCodeCache = Objects.hash(getField(), getReceiver());
}
return hashCodeCache;
Comment on lines 124 to +128
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

equals treats this and super receivers as equivalent in some cases, but hashCode() hashes the concrete receiver via Objects.hash(getField(), getReceiver()). That violates the equals/hashCode contract and can break lookups in hash-based collections. Adjust hashCode() to mirror the equals receiver normalization (e.g., hash a canonical receiver value when it’s ThisReference/SuperReference).

Copilot uses AI. Check for mistakes.
}

@Override
Expand Down
19 changes: 13 additions & 6 deletions dataflow/src/main/java/org/checkerframework/dataflow/expression/FormalParameter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,21 @@ public VariableElement getElement() {
return element;
}

/** Cache the hashCode. Recomputed if zero. */
private int hashCodeCache = 0;

@Override
public int hashCode() {
VarSymbol vs = (VarSymbol) element;
return Objects.hash(
index,
vs.name.toString(),
TypeAnnotationUtils.unannotatedType(vs.type).toString(),
vs.owner.toString());
if (hashCodeCache == 0) {
VarSymbol vs = (VarSymbol) element;
hashCodeCache =
Objects.hash(
index,
vs.name.toString(),
TypeAnnotationUtils.unannotatedType(vs.type).toString(),
vs.owner.toString());
}
return hashCodeCache;
Comment on lines +67 to +81
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hashCodeCache uses 0 as the "not computed" sentinel. If the computed hash ends up 0, Objects.hash(...) will run on every call. Use a separate computed flag (or nullable Integer) so 0 can be cached too.

Copilot uses AI. Check for mistakes.
}

@Override
Expand Down
10 changes: 8 additions & 2 deletions dataflow/src/main/java/org/checkerframework/dataflow/expression/LocalVariable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,16 @@ public VariableElement getElement() {
return element;
}

/** Cache the hashCode. Recomputed if zero. */
private int hashCodeCache = 0;

@Override
public int hashCode() {
VarSymbol vs = (VarSymbol) element;
return Objects.hash(vs.pos, vs.name, vs.owner);
if (hashCodeCache == 0) {
VarSymbol vs = (VarSymbol) element;
hashCodeCache = Objects.hash(vs.pos, vs.name, vs.owner);
}
return hashCodeCache;
Comment on lines +82 to +91
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hashCodeCache uses 0 as the "not computed" sentinel. If Objects.hash(vs.pos, vs.name, vs.owner) evaluates to 0, the hash code will be recomputed every call. Use a separate computed flag (or nullable cache) to cache 0 as a valid value.

Copilot uses AI. Check for mistakes.
}

@Override
Expand Down
15 changes: 11 additions & 4 deletions dataflow/src/main/java/org/checkerframework/dataflow/expression/MethodCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,20 @@ public boolean equals(@Nullable Object obj) {
&& arguments.equals(other.arguments);
}

/** Cache the hashCode. Recomputed if zero. */
private int hashCodeCache = 0;

@Override
public int hashCode() {
if (method.getKind() == ElementKind.CONSTRUCTOR) {
// No two constructor instances have the same hashcode.
return System.identityHashCode(this);
if (hashCodeCache == 0) {
if (method.getKind() == ElementKind.CONSTRUCTOR) {
// No two constructor instances have the same hashcode.
hashCodeCache = System.identityHashCode(this);
} else {
hashCodeCache = Objects.hash(method, receiver, arguments);
}
}
return Objects.hash(method, receiver, arguments);
return hashCodeCache;
Comment on lines 174 to +183
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

equals treats calls on this and super as equal (isComparingSuperWithThis), but hashCode() includes the concrete receiver in Objects.hash(method, receiver, arguments). This violates the equals/hashCode contract and can break map/set usage. Update hashCode() to apply the same receiver equivalence as equals (e.g., canonicalize ThisReference/SuperReference before hashing). Also, using 0 as the cache sentinel means a computed 0 (including a possible identityHashCode of 0) won’t actually be cached.

Copilot uses AI. Check for mistakes.
}

@Override
Expand Down
8 changes: 7 additions & 1 deletion dataflow/src/main/java/org/checkerframework/dataflow/expression/UnaryOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,15 @@ public boolean containsModifiableAliasOf(Store<?> store, JavaExpression other) {
return operand.containsModifiableAliasOf(store, other);
}

/** Cache the hashCode. Recomputed if zero. */
private int hashCodeCache = 0;

@Override
public int hashCode() {
return Objects.hash(operationKind, operand);
if (hashCodeCache == 0) {
hashCodeCache = Objects.hash(operationKind, operand);
}
return hashCodeCache;
Comment on lines +108 to +116
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hashCodeCache uses 0 as the "not computed" sentinel. If the real hash code computes to 0, this will recompute (and re-allocate via Objects.hash(...)) on every call. Prefer a separate boolean (or nullable Integer) so a computed 0 is still cached.

Copilot uses AI. Check for mistakes.
}

@Override
Expand Down
8 changes: 7 additions & 1 deletion dataflow/src/main/java/org/checkerframework/dataflow/expression/ValueLiteral.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,15 @@ public String toString() {
return value == null ? "null" : value.toString();
}

/** Cache the hashCode. Recomputed if zero. */
private int hashCodeCache = 0;

@Override
public int hashCode() {
return Objects.hash(value, type.toString());
if (hashCodeCache == 0) {
hashCodeCache = Objects.hash(value, type.toString());
}
return hashCodeCache;
Comment on lines +166 to +174
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hashCodeCache uses 0 as the "not computed" sentinel. If the real hash code computes to 0, this will recompute (and re-allocate via Objects.hash(...)) on every call, defeating the caching on that path. Use a separate boolean (or an Integer cache) to distinguish "not computed" from a computed 0.

Copilot uses AI. Check for mistakes.
}

@Override
Expand Down
Loading