Skip to content

Commit 9208590

Browse files
klueverError Prone Team
authored andcommitted
Ban using reference equality for MemorySegments.
See openjdk/jdk#30501 PiperOrigin-RevId: 891791638
1 parent 736e704 commit 9208590

4 files changed

Lines changed: 291 additions & 0 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright 2026 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns;
18+
19+
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
20+
import static com.google.errorprone.util.ASTHelpers.getType;
21+
import static com.google.errorprone.util.ASTHelpers.isSubtype;
22+
23+
import com.google.errorprone.BugPattern;
24+
import com.google.errorprone.VisitorState;
25+
import com.google.errorprone.suppliers.Supplier;
26+
import com.sun.source.tree.ExpressionTree;
27+
import com.sun.tools.javac.code.Type;
28+
29+
/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
30+
@BugPattern(summary = "Do not compare MemorySegments using reference equality.", severity = ERROR)
31+
public final class MemorySegmentReferenceEquality extends AbstractReferenceEquality {
32+
33+
private static final Supplier<Type> MEMORY_SEGMENT_TYPE =
34+
VisitorState.memoize(state -> state.getTypeFromString("java.lang.foreign.MemorySegment"));
35+
36+
@Override
37+
protected boolean matchArgument(ExpressionTree tree, VisitorState state) {
38+
Type type = getType(tree);
39+
Type memorySegmentType = MEMORY_SEGMENT_TYPE.get(state);
40+
return (type == null || memorySegmentType == null)
41+
? false
42+
: isSubtype(type, memorySegmentType, state);
43+
}
44+
}

core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@
243243
import com.google.errorprone.bugpatterns.MathAbsoluteNegative;
244244
import com.google.errorprone.bugpatterns.MathRoundIntLong;
245245
import com.google.errorprone.bugpatterns.MemoizeConstantVisitorStateLookups;
246+
import com.google.errorprone.bugpatterns.MemorySegmentReferenceEquality;
246247
import com.google.errorprone.bugpatterns.MethodCanBeStatic;
247248
import com.google.errorprone.bugpatterns.MisformattedTestData;
248249
import com.google.errorprone.bugpatterns.MisleadingEmptyVarargs;
@@ -808,6 +809,7 @@ public static ScannerSupplier warningChecks() {
808809
LoopConditionChecker.class,
809810
LossyPrimitiveCompare.class,
810811
MathRoundIntLong.class,
812+
MemorySegmentReferenceEquality.class,
811813
MislabeledAndroidString.class,
812814
MisleadingEmptyVarargs.class,
813815
MisleadingEscapedSpace.class,
Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
/*
2+
* Copyright 2026 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns;
18+
19+
import static com.google.common.truth.TruthJUnit.assume;
20+
21+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
22+
import org.junit.Before;
23+
import org.junit.Test;
24+
import org.junit.runner.RunWith;
25+
import org.junit.runners.JUnit4;
26+
27+
/**
28+
* @author kak@google.com (Kurt Alfred Kluever)
29+
*/
30+
@RunWith(JUnit4.class)
31+
public class MemorySegmentReferenceEqualityTest {
32+
33+
private final BugCheckerRefactoringTestHelper refactoringHelper =
34+
BugCheckerRefactoringTestHelper.newInstance(MemorySegmentReferenceEquality.class, getClass());
35+
36+
@Before
37+
public void setUp() {
38+
assume().that(Runtime.version().feature()).isAtLeast(22);
39+
}
40+
41+
@Test
42+
public void positiveCase_equal() {
43+
refactoringHelper
44+
.addInputLines(
45+
"Test.java",
46+
"""
47+
import java.lang.foreign.MemorySegment;
48+
49+
class Test {
50+
boolean f(MemorySegment a, MemorySegment b) {
51+
return a == b;
52+
}
53+
}
54+
""")
55+
.addOutputLines(
56+
"Test.java",
57+
"""
58+
import java.lang.foreign.MemorySegment;
59+
import java.util.Objects;
60+
61+
class Test {
62+
boolean f(MemorySegment a, MemorySegment b) {
63+
return Objects.equals(a, b);
64+
}
65+
}
66+
""")
67+
.doTest();
68+
}
69+
70+
@Test
71+
public void positiveCase_notEqual() {
72+
refactoringHelper
73+
.addInputLines(
74+
"Test.java",
75+
"""
76+
import java.lang.foreign.MemorySegment;
77+
78+
class Test {
79+
boolean f(MemorySegment a, MemorySegment b) {
80+
return a != b;
81+
}
82+
}
83+
""")
84+
.addOutputLines(
85+
"Test.java",
86+
"""
87+
import java.lang.foreign.MemorySegment;
88+
import java.util.Objects;
89+
90+
class Test {
91+
boolean f(MemorySegment a, MemorySegment b) {
92+
return !Objects.equals(a, b);
93+
}
94+
}
95+
""")
96+
.doTest();
97+
}
98+
99+
@Test
100+
public void negativeCase_null() {
101+
refactoringHelper
102+
.addInputLines(
103+
"Test.java",
104+
"""
105+
import java.lang.foreign.MemorySegment;
106+
107+
class Test {
108+
boolean f(MemorySegment b) {
109+
return b == null;
110+
}
111+
112+
boolean g(MemorySegment b) {
113+
return b != null;
114+
}
115+
116+
boolean h(MemorySegment b) {
117+
return null == b;
118+
}
119+
120+
boolean i(MemorySegment b) {
121+
return null != b;
122+
}
123+
}
124+
""")
125+
.expectUnchanged()
126+
.doTest();
127+
}
128+
129+
@Test
130+
public void compareToNullConstant() {
131+
refactoringHelper
132+
.addInputLines(
133+
"Test.java",
134+
"""
135+
import java.lang.foreign.MemorySegment;
136+
137+
class Test {
138+
boolean f(MemorySegment a) {
139+
return a == MemorySegment.NULL;
140+
}
141+
142+
boolean g(MemorySegment a) {
143+
return a != MemorySegment.NULL;
144+
}
145+
146+
boolean h(MemorySegment a) {
147+
return MemorySegment.NULL == a;
148+
}
149+
150+
boolean i(MemorySegment a) {
151+
return MemorySegment.NULL != a;
152+
}
153+
}
154+
""")
155+
.addOutputLines(
156+
"Test.java",
157+
"""
158+
import java.lang.foreign.MemorySegment;
159+
import java.util.Objects;
160+
161+
class Test {
162+
boolean f(MemorySegment a) {
163+
return Objects.equals(a, MemorySegment.NULL);
164+
}
165+
166+
boolean g(MemorySegment a) {
167+
return !Objects.equals(a, MemorySegment.NULL);
168+
}
169+
170+
boolean h(MemorySegment a) {
171+
return Objects.equals(MemorySegment.NULL, a);
172+
}
173+
174+
boolean i(MemorySegment a) {
175+
return !Objects.equals(MemorySegment.NULL, a);
176+
}
177+
}
178+
""")
179+
.doTest();
180+
}
181+
182+
@Test
183+
public void compareToNullConstant_notEqual() {
184+
refactoringHelper
185+
.addInputLines(
186+
"Test.java",
187+
"""
188+
import java.lang.foreign.MemorySegment;
189+
190+
class Test {
191+
boolean f(MemorySegment a) {
192+
return a != MemorySegment.NULL;
193+
}
194+
195+
boolean g(MemorySegment a) {
196+
return MemorySegment.NULL != a;
197+
}
198+
}
199+
""")
200+
.addOutputLines(
201+
"Test.java",
202+
"""
203+
import java.lang.foreign.MemorySegment;
204+
import java.util.Objects;
205+
206+
class Test {
207+
boolean f(MemorySegment a) {
208+
return !Objects.equals(a, MemorySegment.NULL);
209+
}
210+
211+
boolean g(MemorySegment a) {
212+
return !Objects.equals(MemorySegment.NULL, a);
213+
}
214+
}
215+
""")
216+
.doTest();
217+
}
218+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
`MemorySegment` is a value-based class. Comparing `MemorySegment` instances
2+
using `==` or `!=` is bug-prone because `MemorySegment` implementations may not
3+
be unique for the same underlying memory. Use `Objects.equals()` (or `.equals()`
4+
if the receiver is known to be non-null) instead.
5+
6+
For example:
7+
8+
```java
9+
MemorySegment seg = ...;
10+
if (seg == MemorySegment.NULL) { // reference equality
11+
...
12+
}
13+
```
14+
15+
should be:
16+
17+
```java
18+
MemorySegment seg = ...;
19+
if (Objects.equals(seg, MemorySegment.NULL)) { // value equality
20+
...
21+
}
22+
```
23+
24+
Reference equality between any two `MemorySegment` instances (e.g., `a == b`) is
25+
flagged by this check, and `Objects.equals(a, b)` is the preferred alternative.
26+
27+
See also https://bugs.openjdk.org/browse/JDK-8381012

0 commit comments

Comments
 (0)