Skip to content

Commit eb18561

Browse files
committed
Filter dependent type annotations referencing out-of-scope local variables
1 parent c6e3865 commit eb18561

4 files changed

Lines changed: 142 additions & 0 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import org.checkerframework.checker.index.qual.LessThan;
2+
import org.checkerframework.checker.index.qual.NonNegative;
3+
4+
/**
5+
* Test case for scope cleanup of dependent type annotations.
6+
*
7+
* <p>When a local variable goes out of scope, any dependent type annotation that references that
8+
* variable should be invalidated. This test verifies that annotations like @LessThan("b") on
9+
* variable 'a' are properly dropped when 'b' goes out of scope.
10+
*/
11+
public class ScopeCleanup {
12+
13+
/**
14+
* Test that @LessThan annotation is properly cleaned up when the referenced variable goes out of
15+
* scope.
16+
*/
17+
void testScopeCleanup() {
18+
int a = 5;
19+
20+
{
21+
int b = 10;
22+
a = b - 1;
23+
// At this point, 'a' should have type @LessThan("b") and that's correct.
24+
25+
// This should be fine: a < b is true
26+
@LessThan("b") int shouldWork = a;
27+
}
28+
29+
// After exiting scope, 'b' is no longer in scope.
30+
// The @LessThan("b") annotation on 'a' should be dropped.
31+
// However, non-dependent annotations should survive.
32+
@NonNegative int ok = a; // This should still work - a is still non-negative (a=9)
33+
34+
{
35+
int b = 3; // Different variable 'b' with a smaller value!
36+
// Without the fix, the old @LessThan("b") annotation would incorrectly refer to THIS 'b'.
37+
// But a = 9 and this b = 3, so a > b, not a < b.
38+
39+
// This should be an error - we can no longer prove a < b
40+
// The assignment below should fail type-checking because we can't prove a < b
41+
// :: error: (assignment)
42+
@LessThan("b") int shouldFail = a;
43+
}
44+
}
45+
46+
/**
47+
* Test that method parameters are not affected by scope cleanup (they should always be considered
48+
* in scope).
49+
*/
50+
void testParametersRemainInScope(@NonNegative int param) {
51+
int x;
52+
{
53+
int localVar = param + 1;
54+
x = param - 1; // x < param should be valid
55+
}
56+
// Even after the block ends, 'param' is still in scope, so @LessThan("param") should be valid
57+
// This test ensures method parameters are handled correctly
58+
}
59+
}

framework/src/main/java/org/checkerframework/framework/flow/CFAbstractStore.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
import java.util.ArrayList;
44
import java.util.Collection;
5+
import java.util.Collections;
56
import java.util.HashMap;
67
import java.util.Iterator;
78
import java.util.List;
89
import java.util.Map;
910
import java.util.Objects;
11+
import java.util.Set;
1012
import java.util.StringJoiner;
1113
import java.util.concurrent.atomic.AtomicLong;
1214
import java.util.function.BinaryOperator;
@@ -1141,6 +1143,17 @@ public boolean canAlias(JavaExpression a, JavaExpression b) {
11411143
return localVariableValues.get(new LocalVariable(el));
11421144
}
11431145

1146+
/**
1147+
* Returns the set of local variables currently tracked in this store.
1148+
*
1149+
* <p>Clients should not side-effect the returned value, which is aliased to internal state.
1150+
*
1151+
* @return the set of local variables currently in scope
1152+
*/
1153+
public Set<LocalVariable> getLocalVariables() {
1154+
return Collections.unmodifiableSet(localVariableValues.keySet());
1155+
}
1156+
11441157
/* --------------------------------------------------------- */
11451158
/* Handling of the current object */
11461159
/* --------------------------------------------------------- */

framework/src/main/java/org/checkerframework/framework/type/GenericAnnotatedTypeFactory.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1989,6 +1989,16 @@ protected void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type, b
19891989
log(
19901990
"%s GATF.addComputedTypeAnnotations#8(%s, %s), inferred=%s%n",
19911991
thisClass, treeString, type, inferred);
1992+
1993+
// Filter out dependent type annotations that reference out-of-scope local variables
1994+
if (dependentTypesHelper.hasDependentAnnotations()) {
1995+
Store store = getStoreBefore(tree);
1996+
if (store != null) {
1997+
Set<LocalVariable> inScopeVars = store.getLocalVariables();
1998+
dependentTypesHelper.filterAnnotationsForOutOfScopeVars(
1999+
type, inScopeVars, getPath(tree));
2000+
}
2001+
}
19922002
}
19932003
}
19942004
log(

framework/src/main/java/org/checkerframework/framework/util/dependenttypes/DependentTypesHelper.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,66 @@ public JavaExpression visitSuperReference(SuperReference superRef, Void unused)
755755
convertAnnotatedTypeMirror(stringToJavaExpr, atm);
756756
}
757757

758+
/**
759+
* Filters dependent type annotations in {@code type}, removing those that reference local
760+
* variables not currently in scope.
761+
*
762+
* <p>When a local variable goes out of scope, any dependent type annotation that references that
763+
* variable becomes invalid and should be dropped. This method checks each expression in dependent
764+
* type annotations and drops expressions that reference local variables not in {@code
765+
* inScopeLocalVars}.
766+
*
767+
* @param type the type whose annotations may be filtered; is side-effected by this method
768+
* @param inScopeLocalVars the set of local variables currently in scope
769+
* @param path the tree path for expression parsing context; may be null
770+
*/
771+
public void filterAnnotationsForOutOfScopeVars(
772+
AnnotatedTypeMirror type, Set<LocalVariable> inScopeLocalVars, TreePath path) {
773+
if (!hasDependentType(type)) {
774+
return;
775+
}
776+
777+
if (path == null) {
778+
return;
779+
}
780+
781+
StringToJavaExpression stringToJavaExpr =
782+
expression -> {
783+
// Check if this should be passed through unchanged
784+
if (shouldPassThroughExpression(expression)) {
785+
return new PassThroughExpression(objectTM, expression);
786+
}
787+
788+
JavaExpression javaExpr =
789+
StringToJavaExpression.atPath(expression, path, factory.getChecker());
790+
791+
// Check if the expression references any local variable not in scope
792+
JavaExpressionConverter jec =
793+
new JavaExpressionConverter() {
794+
@Override
795+
protected JavaExpression visitLocalVariable(LocalVariable localVarExpr, Void p) {
796+
// Check if this local variable is a method parameter
797+
if (localVarExpr.getElement().getKind() == ElementKind.PARAMETER) {
798+
return localVarExpr; // Parameters are always in scope
799+
}
800+
// Check if the local variable is in scope
801+
if (!inScopeLocalVars.contains(localVarExpr)) {
802+
throw new FoundLocalVarException();
803+
}
804+
return localVarExpr;
805+
}
806+
};
807+
808+
try {
809+
return jec.convert(javaExpr);
810+
} catch (FoundLocalVarException ex) {
811+
return null; // Drop this expression - variable is out of scope
812+
}
813+
};
814+
815+
convertAnnotatedTypeMirror(stringToJavaExpr, type);
816+
}
817+
758818
/**
759819
* Calls {@link #convertAnnotationMirror(StringToJavaExpression, AnnotationMirror)} on each
760820
* annotation mirror on type with {@code stringToJavaExpr}. And replaces the annotation with the

0 commit comments

Comments
 (0)