Skip to content

Add downcast to CF #213

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

Closed
wants to merge 15 commits into from
Closed

Add downcast to CF #213

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0b4910b
add downcast to cf
AndrewShf Jun 14, 2022
f765a98
add downcast to cf
AndrewShf Jun 15, 2022
5fbe6e2
add downcast/incomparable cast to cf
AndrewShf Jun 17, 2022
3e7383b
use getEffectiveAnnotation
AndrewShf Jun 18, 2022
2febfb9
check the java type down cast
AndrewShf Jun 18, 2022
67a2ed2
revert typo fix, fix in another pr
AndrewShf Jun 19, 2022
b52fb43
add incomparable cast message key
AndrewShf Jun 22, 2022
9de934d
replace some warnings with incompatible cast errors
AndrewShf Jun 23, 2022
78ecdeb
refined the approach when casting T to T
AndrewShf Jun 25, 2022
87d3d74
suppress some warnings
AndrewShf Jun 25, 2022
53a732d
refine the logic to cast from T to T
AndrewShf Jun 26, 2022
d31ef8f
add method documentation
AndrewShf Jun 26, 2022
0e2fac3
remove useless for loop
AndrewShf Aug 8, 2022
cda3b63
Merge remote-tracking branch 'opprop/master' into add_down_incomparab…
AndrewShf Sep 12, 2022
3aad902
update mustcallvisitor, signedness visitor
AndrewShf Sep 12, 2022
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
3 changes: 2 additions & 1 deletion checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ protected void commonAssignmentCheck(
}

@Override
protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
protected CastSafeKind isTypeCastSafe(
AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {

AnnotationMirror exprLTAnno =
exprType.getEffectiveAnnotationInHierarchy(atypeFactory.LESS_THAN_UNKNOWN);
Expand Down
5 changes: 3 additions & 2 deletions checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,9 +977,10 @@ DeclaredType typeToCheck() {
}

@Override
protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
protected CastSafeKind isTypeCastSafe(
AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
if (castType.getKind().isPrimitive()) {
return true;
return CastSafeKind.SAFE;
}
return super.isTypeCastSafe(castType, exprType);
}
Expand Down
173 changes: 135 additions & 38 deletions framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2327,41 +2327,42 @@ protected void checkTypecastSafety(TypeCastTree typeCastTree) {
}
// We cannot do a simple test of casting, as isSubtypeOf requires
// the input types to be subtypes according to Java.
if (!calledOnce && !isTypeCastSafe(castType, exprType)) {
checker.reportWarning(
typeCastTree, "cast.unsafe", exprType.toString(true), castType.toString(true));
if (!calledOnce) {
CastSafeKind castResult = isTypeCastSafe(castType, exprType);

if (castResult == CastSafeKind.WARNING
|| castResult == CastSafeKind.ERROR) { // TODO: refine the error report
checker.reportWarning(
typeCastTree,
"cast.unsafe",
exprType.toString(true),
castType.toString(true));
}
}
}

/** CastSafeKind */
protected enum CastSafeKind {
SAFE,
ERROR,
WARNING,
NOT_UPCAST,
NOT_DOWNCAST
}

/**
* Returns true if the cast is safe.
* isUpcast
*
* <p>Only primary qualifiers are checked unless the command line option "checkCastElementType"
* is supplied.
*
* @param castType annotated type of the cast
* @param exprType annotated type of the casted expression
* @return true if the type cast is safe, false otherwise
* @param castType castType
* @param exprType exprType
* @return CastSafeKind
*/
protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {

final TypeKind castTypeKind = castType.getKind();
if (castTypeKind == TypeKind.DECLARED) {
// Don't issue an error if the annotations are equivalent to the qualifier upper bound
// of the type.
AnnotatedDeclaredType castDeclared = (AnnotatedDeclaredType) castType;
Set<AnnotationMirror> bounds =
atypeFactory.getTypeDeclarationBounds(castDeclared.getUnderlyingType());

if (AnnotationUtils.areSame(castDeclared.getAnnotations(), bounds)) {
return true;
}
}

protected CastSafeKind isUpcast(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
QualifierHierarchy qualifierHierarchy = atypeFactory.getQualifierHierarchy();

Set<AnnotationMirror> castAnnos;
if (!checker.hasOption("checkCastElementType")) {
TypeKind castTypeKind = castType.getKind();
boolean flagEnabled = checker.hasOption("checkCastElementType");
if (!flagEnabled) {
// checkCastElementType option wasn't specified, so only check effective annotations.
castAnnos = castType.getEffectiveAnnotations();
} else {
Expand All @@ -2379,13 +2380,13 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr
}

if (!atypeFactory.getTypeHierarchy().isSubtype(newExprType, newCastType)) {
return false;
return CastSafeKind.ERROR;
}
if (newCastType.getKind() == TypeKind.ARRAY
&& newExprType.getKind() != TypeKind.ARRAY) {
// Always warn if the cast contains an array, but the expression
// doesn't, as in "(Object[]) o" where o is of type Object
return false;
return CastSafeKind.ERROR;
} else if (newCastType.getKind() == TypeKind.DECLARED
&& newExprType.getKind() == TypeKind.DECLARED) {
int castSize = ((AnnotatedDeclaredType) newCastType).getTypeArguments().size();
Expand All @@ -2395,7 +2396,7 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr
// Always warn if the cast and expression contain a different number of type
// arguments, e.g. to catch a cast from "Object" to "List<@NonNull Object>".
// TODO: the same number of arguments actually doesn't guarantee anything.
return false;
return CastSafeKind.ERROR;
}
} else if (castTypeKind == TypeKind.TYPEVAR && exprType.getKind() == TypeKind.TYPEVAR) {
// If both the cast type and the casted expression are type variables, then check
Expand All @@ -2406,11 +2407,13 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr
Set<AnnotationMirror> lowerBoundAnnotationsExpr =
AnnotatedTypes.findEffectiveLowerBoundAnnotations(
qualifierHierarchy, exprType);
return qualifierHierarchy.isSubtype(
lowerBoundAnnotationsExpr, lowerBoundAnnotationsCast)
&& qualifierHierarchy.isSubtype(
exprType.getEffectiveAnnotations(),
castType.getEffectiveAnnotations());
boolean result =
qualifierHierarchy.isSubtype(
lowerBoundAnnotationsExpr, lowerBoundAnnotationsCast)
&& qualifierHierarchy.isSubtype(
exprType.getEffectiveAnnotations(),
castType.getEffectiveAnnotations());
return result ? CastSafeKind.SAFE : CastSafeKind.ERROR;
}
if (castTypeKind == TypeKind.TYPEVAR) {
// If the cast type is a type var, but the expression is not, then check that the
Expand All @@ -2423,8 +2426,102 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr
}
}

// // Return a warning in this case, as compiler will not report the unchecked
// warning in
// // this case, and we cannot statically verify the subtype relation of the
// annotations.
// if (castTypeKind == TypeKind.TYPEVAR && exprType.getKind() == TypeKind.TYPEVAR) {
// TypeMirror castJavaType = castType.getUnderlyingType();
// TypeMirror exprJavaType = exprType.getUnderlyingType();
// if (TypesUtils.areSameTypeVariables(castJavaType, exprJavaType)) {
// return CastSafeKind.WARNING;
// }
// }

AnnotatedTypeMirror exprTypeWidened = atypeFactory.getWidenedType(exprType, castType);
return qualifierHierarchy.isSubtype(exprTypeWidened.getEffectiveAnnotations(), castAnnos);
boolean result =
qualifierHierarchy.isSubtype(exprTypeWidened.getEffectiveAnnotations(), castAnnos);

if (result) {
return CastSafeKind.SAFE;
} else if (flagEnabled) { // when the flag is enabled and it is not an upcast, return an
// error
return CastSafeKind.ERROR;
} else {
return CastSafeKind.NOT_UPCAST;
}
}

/**
* isSafeDowncast
*
* @param castType castType
* @param exprType exprType
* @return CastSafeKind
*/
protected CastSafeKind isSafeDowncast(
AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
Set<AnnotationMirror> castAnnos = castType.getEffectiveAnnotations();
Set<AnnotationMirror> exprAnnos = exprType.getEffectiveAnnotations();
QualifierHierarchy qualifierHierarchy = atypeFactory.getQualifierHierarchy();

if (!qualifierHierarchy.isSubtype(castAnnos, exprAnnos)) { // not a downcast
return CastSafeKind.NOT_DOWNCAST;
}

// check if the downcast can be statically verified
final TypeKind castTypeKind = castType.getKind();

if (castTypeKind == TypeKind.DECLARED) {
// Don't issue an error if the annotations are equivalent to the qualifier upper bound
// of the type.
AnnotatedDeclaredType castDeclared = (AnnotatedDeclaredType) castType;
Set<AnnotationMirror> bounds =
atypeFactory.getTypeDeclarationBounds(castDeclared.getUnderlyingType());

if (AnnotationUtils.areSame(castDeclared.getAnnotations(), bounds)) {
TypeMirror castJavaType = castType.getUnderlyingType();
TypeMirror exprJavaType = exprType.getUnderlyingType();
if (TypesUtils.isErasedSubtype(castJavaType, exprJavaType, types)) {
return CastSafeKind.SAFE;
}
}
}
return CastSafeKind.WARNING;
}

protected CastSafeKind isSafeIncomparableCast(
AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
return CastSafeKind.ERROR;
}

/**
* Returns true if the cast is safe.
*
* <p>Only primary qualifiers are checked unless the command line option "checkCastElementType"
* is supplied.
*
* @param castType annotated type of the cast
* @param exprType annotated type of the casted expression
* @return CastSafeKind if the type cast is safe, false otherwise
*/
protected CastSafeKind isTypeCastSafe(
AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) {
CastSafeKind castResult = isUpcast(castType, exprType);

if (castResult
== CastSafeKind.NOT_UPCAST) { // not upcast, do downcast and incomparable cast check
castResult = isSafeDowncast(castType, exprType);

if (castResult == CastSafeKind.NOT_DOWNCAST) { // fall to incomparable cast
return isSafeIncomparableCast(castType, exprType);
} else {
return castResult;
}

} else {
return castResult;
}
}

/**
Expand All @@ -2437,7 +2534,7 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr
*/
private boolean isInvariantTypeCastSafe(
AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType, AnnotationMirror top) {
if (!isTypeCastSafe(castType, exprType)) {
if (isTypeCastSafe(castType, exprType) != CastSafeKind.SAFE) {
return false;
}
AnnotationMirror castTypeAnno = castType.getEffectiveAnnotationInHierarchy(top);
Expand Down Expand Up @@ -2489,7 +2586,7 @@ public Void visitInstanceOf(InstanceOfTree tree, Void p) {
if (variableTree.getModifiers() != null) {
AnnotatedTypeMirror variableType = atypeFactory.getAnnotatedType(variableTree);
AnnotatedTypeMirror expType = atypeFactory.getAnnotatedType(tree.getExpression());
if (!isTypeCastSafe(variableType, expType)) {
if (isTypeCastSafe(variableType, expType) != CastSafeKind.SAFE) {
checker.reportWarning(tree, "instanceof.pattern.unsafe", expType, variableTree);
}
}
Expand Down
12 changes: 12 additions & 0 deletions javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,18 @@ public static boolean areSamePrimitiveTypes(TypeMirror left, TypeMirror right) {
return (left.getKind() == right.getKind());
}

/**
* Returns true iff the arguments are both the same type variables.
*
* @param t1 a type
* @param t2 a type
* @return whether the arguments are the same type variables
*/
public static boolean areSameTypeVariables(
TypeMirror t1, TypeMirror t2) { // TODO: a cheaper way of doing this?
return t1.toString().equals(t2.toString());
}

/// Predicates

/**
Expand Down