Improve cast warnings/errors logic

checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanVisitor.java

@@ -117,7 +117,8 @@ protected boolean commonAssignmentCheck(
     }
 
     @Override
-    protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
+    protected TypecastKind isTypeCastSafe(
+            AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
 
         AnnotationMirror exprLTAnno =
                 exprType.getEffectiveAnnotationInHierarchy(atypeFactory.LESS_THAN_UNKNOWN);

checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java

@@ -984,9 +984,10 @@ private boolean overrides(ExecutableElement e, Class<?> clazz, String method) {
     }
 
     @Override
-    protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
+    protected TypecastKind isTypeCastSafe(
+            AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
         if (castType.getKind().isPrimitive()) {
-            return true;
+            return TypecastKind.SAFE;
         }
         return super.isTypeCastSafe(castType, exprType);
     }

checker/src/main/java/org/checkerframework/checker/signedness/SignednessVisitor.java

@@ -379,10 +379,11 @@ public Void visitCompoundAssignment(CompoundAssignmentTree tree, Void p) {
     }
 
     @Override
-    protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
+    protected TypecastKind isTypeCastSafe(
+            AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
         if (!atypeFactory.maybeIntegral(castType)) {
             // If the cast is not a number or a char, then it is legal.
-            return true;
+            return TypecastKind.SAFE;
         }
         return super.isTypeCastSafe(castType, exprType);
     }

checker/tests/initialization/CastInit.java

@@ -5,7 +5,7 @@ public class CastInit {
 
     public CastInit() {
         @UnknownInitialization CastInit t1 = (@UnknownInitialization CastInit) this;
-        // :: warning: (cast.unsafe)
+        // :: error: (cast.incomparable)
         @Initialized CastInit t2 = (@Initialized CastInit) this;
     }
 }

checker/tests/lock/ChapterExamples.java

@@ -319,7 +319,7 @@ void someMethod() {
         o2 = o1; // {"lock"} and {} are not identical sets.
     }
 
-    @SuppressWarnings("lock:cast.unsafe")
+    @SuppressWarnings("lock:cast.incomparable")
     void someMethod2() {
         // A cast can be used if the user knows it is safe to do so.
         // However, the @SuppressWarnings must be added.
@@ -567,7 +567,7 @@ public boolean compare(T[] a1, T[] a2) {
     private static final Object NULL_KEY = new Object();
 
     // A guardsatisfied.location.disallowed error is issued for the cast.
-    @SuppressWarnings({"cast.unsafe", "guardsatisfied.location.disallowed"})
+    @SuppressWarnings({"cast.incomparable", "guardsatisfied.location.disallowed"})
     private static @GuardSatisfied(1) Object maskNull(@GuardSatisfied(1) Object key) {
         return (key == null ? (@GuardSatisfied(1) Object) NULL_KEY : key);
     }

checker/tests/lock/Strings.java

@@ -13,7 +13,9 @@ void StringIsGBnothing(
             @GuardSatisfied Object o3,
             @GuardedByBottom Object o4) {
         String s1 = (String) o1;
+        // :: error: (cast.incomparable)
         String s2 = (String) o2;
+        // :: error: (cast.incomparable)
         String s3 = (String) o3;
         String s4 = (String) o4; // OK
     }

checker/tests/signedness/CastFromTtoT.java

@@ -0,0 +1,41 @@
+import org.checkerframework.checker.signedness.qual.Signed;
+import org.checkerframework.checker.signedness.qual.SignednessGlb;
+import org.checkerframework.checker.signedness.qual.UnknownSignedness;
+
+class CastFromTtoT<T extends @UnknownSignedness Object> {
+    @SuppressWarnings("unchecked")
+    T bar(@UnknownSignedness Object p) {
+        // Seems to have no cast in terms of the qualifier (from @UnknownSignedness to
+        // @UnknownSignedness), but in instantiation, it could be a downcast.
+        // See method foo below. It's okay not to report downcast warnings as Javac will warn about
+        // casting object to 'T' (unchecked warning)
+        T x = (T) p;
+        return x;
+    }
+
+    void foo(CastFromTtoT<@Signed Integer> s, @UnknownSignedness Object local) {
+        // Here, we passed in an @UnknownSignedness object and the method signature after
+        // substitution is @Signed Integer bar(@UnknownSignedness Object). This makes the typecast
+        // discussed earlier a downcast.
+        @Signed Integer x = s.bar(local);
+    }
+
+    class Inner<T extends @UnknownSignedness Object> {
+        T bar2(@Signed T p) {
+            // The casting expression below looks like an upcast (in terms of the qualifier),
+            // but it could be a downcast in invocation (See method foo2 below for an example).
+            // We should report downcast warning if there is one because
+            // Javac doesn't warn when casting a variable from type T to T.
+            // :: warning: (cast.unsafe)
+            T x = (T) p;
+            return x;
+        }
+
+        void foo2(Inner<@SignednessGlb Integer> s, @Signed Integer local) {
+            // Here, we passed in an @Signed integer and the method signature after
+            // substitution is @SignednessGlb Integer bar2(@Signed Object). This makes the typecast
+            // discussed in method bar2 a downcast.
+            @SignednessGlb Integer x = s.bar2(local);
+        }
+    }
+}

checker/tests/signedness/LiteralCast.java

@@ -24,7 +24,7 @@ void m() {
         requireSigned((@Unsigned int) 2);
         requireSigned((int) 2);
         requireSigned((@m int) 2);
-        // :: warning: (cast.unsafe)
+        // :: error: (cast.incomparable)
         requireSigned((@Signed int) u);
         // :: error: (argument.type.incompatible)
         requireSigned((@Unsigned int) u);
@@ -33,7 +33,7 @@ void m() {
         // :: error: (argument.type.incompatible)
         requireSigned((@m int) u);
         requireSigned((@Signed int) s);
-        // :: error: (argument.type.incompatible) :: warning: (cast.unsafe)
+        // :: error: (argument.type.incompatible) :: error: (cast.incomparable)
         requireSigned((@Unsigned int) s);
         requireSigned((int) s);
         requireSigned((@m int) s);
@@ -43,14 +43,14 @@ void m() {
         requireUnsigned((@Unsigned int) 2);
         requireUnsigned((int) 2);
         requireUnsigned((@m int) 2);
-        // :: error: (argument.type.incompatible) :: warning: (cast.unsafe)
+        // :: error: (argument.type.incompatible) :: error: (cast.incomparable)
         requireUnsigned((@Signed int) u);
         requireUnsigned((@Unsigned int) u);
         requireUnsigned((int) u);
         requireUnsigned((@m int) u);
         // :: error: (argument.type.incompatible)
         requireUnsigned((@Signed int) s);
-        // :: warning: (cast.unsafe)
+        // :: error: (cast.incomparable)
         requireUnsigned((@Unsigned int) s);
         // :: error: (argument.type.incompatible)
         requireUnsigned((int) s);

docs/CHANGELOG.md

@@ -3,11 +3,18 @@ Version 3.42.0-eisop6 (January ??, 2025)
 
 **User-visible changes:**
 
+A new error message `cast.incomparable` will be raised if the target type qualifier is neither the subtype
+nor the supertype of the expression type qualifier. No longer issue errors for statically verifiable downcast.
+
 **Implementation details:**
 
+Refactored the implementation of `isTypeCastSafe` to categorize the kinds of a typecast, whether
+it is an upcast, downcast or incomparable cast. Based on that, further determine if the typecast
+is statically verifiable or not.
+
 **Closed issues:**
 
-eisop#1003, eisop#1033.
+eisop#155, eisop#1003, eisop#1033.
 
 
 Version 3.42.0-eisop5 (December 20, 2024)
@@ -188,7 +195,6 @@ is an enhanced switch statement.
 
 eisop#609, eisop#610, eisop#612.
 
-
 Version 3.40.0 (November 1, 2023)
 ---------------------------------