Skip to content
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.

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

Add lost qualifier to viewpointtest type checker #827

Open
wants to merge 38 commits into
base: master
Choose a base branch
from

Conversation

Ao-senXiong
Copy link
Member

@Ao-senXiong Ao-senXiong commented Aug 3, 2024

Fixes #576

@Ao-senXiong
Copy link
Member Author

Fixes #576

TODO: lost is not reflexive

@Ao-senXiong
Copy link
Member Author

Fixes #576

TODO: lost is not reflexive

This is implemented in 76a9f40, but how to improve the error message remain as a questions.

Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@wmdietl wmdietl assigned Ao-senXiong and unassigned wmdietl Aug 19, 2024
@Ao-senXiong Ao-senXiong removed their assignment Aug 27, 2024
@Ao-senXiong Ao-senXiong assigned wmdietl and unassigned Ao-senXiong Oct 3, 2024
Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes and the extensive tests!

@@ -2,6 +2,7 @@
receiver.invalid=incompatible types.%nfound : %s%nrequired: %s
type.anno.before.modifier=write type annotation %s immediately before type, after modifiers %s
type.anno.before.decl.anno=write type annotations %s immediately before type, after declaration annotation %s
new.class.type.invalid=the annotations %s do not need be applied in object creations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this further down to other type.invalid error messages.
While we're at it, can you improve the grammar?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@PolyVP MyClass(@PolyVP Object o) {
throw new RuntimeException(" * You are filled with DETERMINATION."); // stub
// :: warning: (cast.unsafe.constructor.invocation)
throw new @A RuntimeException(" * You are filled with DETERMINATION."); // stub
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of this change? Should there be multiple versions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expand the test suite in here.

VarargsConstructor a = new VarargsConstructor("testStr", new Object());
VarargsConstructor a =
// :: warning: (cast.unsafe.constructor.invocation)
new @A VarargsConstructor("testStr", new @A Object());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change illustrating something about Lost? Should there be versions of this test without an annotation? A comment would help understand this test.

Copy link
Member Author

@Ao-senXiong Ao-senXiong Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not about lost, I update this test case because I want those invalid invocation because previous we allow @Top constructor invocation. More test cases for lost is in LostNonReflexive.java

@Override
public Void visitNewClass(NewClassTree tree, Void p) {
AnnotatedTypeMirror Type = atypeFactory.getAnnotatedType(tree);
if (Type.hasAnnotation(atypeFactory.TOP)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't LOST also be forbidden?

Copy link
Member Author

@Ao-senXiong Ao-senXiong Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, how about poly and RD qualifier? I left out those two as I feel like we need more discussion.

@wmdietl wmdietl assigned Ao-senXiong and unassigned wmdietl Nov 13, 2024
Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@wmdietl wmdietl enabled auto-merge (squash) December 4, 2024 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lost qualifier should be added to type systems with viewpoint adaptation functionality.
2 participants