Handle intersection type in viewpoint adaptation#1434
Handle intersection type in viewpoint adaptation#1434aosen-xiong wants to merge 7 commits intoeisop:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a Checker Framework crash (#433) when viewpoint adaptation encounters intersection types by adapting each bound of an INTERSECTION type and adds a regression test.
Changes:
- Handle
TypeKind.INTERSECTIONinAbstractViewpointAdapter.combineAnnotationWithType. - Handle
TypeKind.INTERSECTIONinAbstractViewpointAdapter.substituteTVars. - Add a
viewpointtestregression test for intersection-type adaptation and update the changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| framework/tests/viewpointtest/IntersectionTypes.java | Adds regression test coverage for intersection-type viewpoint adaptation. |
| framework/src/main/java/org/checkerframework/framework/type/AbstractViewpointAdapter.java | Adds intersection-type handling during viewpoint adaptation and type-variable substitution. |
| docs/CHANGELOG.md | Notes the closed issue (#433). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| class E extends D<BC> {} | ||
|
|
||
| <T extends B<T> & C<T>> void call(T p) {} |
There was a problem hiding this comment.
This file defines call(T p) twice (line 14 and again at line 32) with identical erasure/signature; Java will reject this as a duplicate method declaration. Rename one of the methods (or change its parameter list) so the test compiles and can exercise both intersection-type scenarios.
| <T extends B<T> & C<T>> void call(T p) {} | |
| <T extends B<T> & C<T>> void callBC(T p) {} |
| AnnotatedTypeMirror.AnnotatedIntersectionType intersection = | ||
| (AnnotatedTypeMirror.AnnotatedIntersectionType) declared.shallowCopy(true); | ||
| List<AnnotatedTypeMirror> listBounds = intersection.getBounds(); | ||
| List<AnnotatedTypeMirror> listBoundsCopy = new ArrayList<>(listBounds); | ||
| for (int i = 0; i < listBoundsCopy.size(); i++) { | ||
| AnnotatedTypeMirror bound = listBoundsCopy.get(i); | ||
| AnnotatedTypeMirror combinedBound = | ||
| combineAnnotationWithType(receiverAnnotation, bound); | ||
| listBoundsCopy.set(i, combinedBound); | ||
| } | ||
| intersection.setBounds(listBoundsCopy); |
There was a problem hiding this comment.
AnnotatedIntersectionType.shallowCopy(true) copies primary annotations via addAnnotations, and AnnotatedIntersectionType.addAnnotation propagates them to the (shared) bounds list. Because shallowCopy also shares bounds (type.bounds = this.bounds), this can mutate the original declared bounds, and the returned intersection's primary annotations can become inconsistent after you replace bounds with viewpoint-adapted ones. To keep this method side-effect free and consistent, construct the copy without copying annotations (e.g., shallowCopy(false) or deepCopy(false)), adapt bounds, set them, then recompute primary annotations from the new bounds (e.g., clearAnnotations() + copyIntersectionBoundAnnotations()).
| AnnotatedTypeMirror.AnnotatedIntersectionType intersection = | |
| (AnnotatedTypeMirror.AnnotatedIntersectionType) declared.shallowCopy(true); | |
| List<AnnotatedTypeMirror> listBounds = intersection.getBounds(); | |
| List<AnnotatedTypeMirror> listBoundsCopy = new ArrayList<>(listBounds); | |
| for (int i = 0; i < listBoundsCopy.size(); i++) { | |
| AnnotatedTypeMirror bound = listBoundsCopy.get(i); | |
| AnnotatedTypeMirror combinedBound = | |
| combineAnnotationWithType(receiverAnnotation, bound); | |
| listBoundsCopy.set(i, combinedBound); | |
| } | |
| intersection.setBounds(listBoundsCopy); | |
| AnnotatedTypeMirror.AnnotatedIntersectionType declaredIntersection = | |
| (AnnotatedTypeMirror.AnnotatedIntersectionType) declared; | |
| AnnotatedTypeMirror.AnnotatedIntersectionType intersection = | |
| (AnnotatedTypeMirror.AnnotatedIntersectionType) | |
| declaredIntersection.shallowCopy(/* copyAnnotations= */ false); | |
| List<AnnotatedTypeMirror> listBounds = declaredIntersection.getBounds(); | |
| List<AnnotatedTypeMirror> listBoundsCopy = new ArrayList<>(listBounds.size()); | |
| for (AnnotatedTypeMirror bound : listBounds) { | |
| AnnotatedTypeMirror combinedBound = | |
| combineAnnotationWithType(receiverAnnotation, bound); | |
| listBoundsCopy.add(combinedBound); | |
| } | |
| intersection.setBounds(listBoundsCopy); | |
| intersection.clearAnnotations(); | |
| intersection.copyIntersectionBoundAnnotations(); |
| AnnotatedTypeMirror.AnnotatedIntersectionType intersection = | ||
| (AnnotatedTypeMirror.AnnotatedIntersectionType) rhs.shallowCopy(true); | ||
| List<AnnotatedTypeMirror> listBounds = intersection.getBounds(); | ||
| List<AnnotatedTypeMirror> listBoundsCopy = new ArrayList<>(listBounds); | ||
| for (int i = 0; i < listBoundsCopy.size(); i++) { | ||
| AnnotatedTypeMirror bound = listBoundsCopy.get(i); | ||
| AnnotatedTypeMirror substBound = substituteTVars(lhs, bound); | ||
| listBoundsCopy.set(i, substBound); | ||
| } | ||
| intersection.setBounds(listBoundsCopy); | ||
| rhs = intersection; |
There was a problem hiding this comment.
substituteTVars is documented as side-effect free, but rhs.shallowCopy(true) on an intersection shares its bounds list and copies annotations in a way that propagates to those shared bounds, potentially mutating the original rhs. Prefer creating the intersection copy without copying annotations (or use deepCopy(true)), then substitute bounds and (if needed) ensure the intersection's primary annotations remain consistent with its bounds.
|
@aosen-xiong Can you look at the Copilot comments and address or resolve them? |
Fixes #433 by adapting each bound from the intersection type and reset them back to
AnnotatedIntersectionType.