Fix viewpoint-adapted varargs type#903
Conversation
wmdietl
left a comment
There was a problem hiding this comment.
Can you add some test cases that illustrate what goes wrong without this change?
f16f289 to
c2b1bf1
Compare
|
This does not fix #828, it is a very deep issue that requires hard investigation. |
There was a problem hiding this comment.
Pull request overview
Fixes EISOP issue #386 by ensuring that an AnnotatedExecutableType’s cached varargs type is recomputed after viewpoint adaptation mutates the executable’s parameter types, so varargs checking uses the adapted (correct) type.
Changes:
- Recompute
varargTypeafter viewpoint adaptation inAbstractViewpointAdapterfor both constructors and methods. - Update/extend viewpoint varargs tests to expect
varargs.type.incompatiblewhere appropriate. - Add a dedicated regression test for issue #386 and record the fix in the changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| framework/tests/viewpointtest/VarargsConstructor.java | Updates expected diagnostics to include varargs.type.incompatible after the fix. |
| framework/tests/viewpointtest/Issue386.java | Adds regression coverage for viewpoint-adapted varargs on constructors and methods. |
| framework/src/main/java/org/checkerframework/framework/type/AbstractViewpointAdapter.java | Recomputes cached varargs type after updating executable parameter types via viewpoint adaptation. |
| docs/CHANGELOG.md | Notes closure of #386. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| void foo() { | ||
| // :: error: (new.class.type.invalid) | ||
| // :: error: (new.class.type.invalid) :: error: (varargs.type.incompatible) |
There was a problem hiding this comment.
What is the intuition behind this new error? There are no arguments to this constructor invocation, so what is incompatible?
There was a problem hiding this comment.
The err msg is:
VarargsConstructor.java:35: error: [new.class.type.invalid]
type annotation [@Lost] can not be applied to object creation
VarargsConstructor.java:35: error: [varargs.type.incompatible]
incompatible types in varargs.
found : @Lost Object @Top []
required: @Lost Object @Top []
A zero-argument varargs invocation still creates an implicit empty array. The error occurs because viewpoint adaptation produces @Lost, which is non-reflexive in this test checker. With #1173, the error would be gone.
There was a problem hiding this comment.
What is the connection to #1173? 1173 is about lost receivers for poly declared receiver methods. This is about arrays?
If the var args array is definitely empty, the Lost component type cannot do any harm. It seems overly conservative to forbid this call, doesn't it?
There was a problem hiding this comment.
Because in #1173 we changed Lost to be a subtype of Lost, this varags.type.incompatible error message will be gone.
Okay, I see your points.
I think even after the change in #1173, we should have
- If the argument is empty, allow the call.
- If the argument is not empty and the var arg is adapted to have a lost type, reject the call.
Fixes #386
Recompute the cached varargs type after viewpoint adaptation updates executable parameter types.