-
Notifications
You must be signed in to change notification settings - Fork 19
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
Record visited type to prevent stack overflow #788
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, similar code that uses wildcards instead of raw types still crashes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
RawtypeInstantiation<?> wildcardInstance = new @A RawtypeInstantiation<C>(); | ||
} | ||
|
||
<D extends RawtypeInstantiation<D>> void bar(D b) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the parameter b
used for something? Or can it be removed?
// https://github.com/eisop/checker-framework/issues/778 | ||
import viewpointtest.quals.*; | ||
|
||
public class RawtypeInstantiation<C extends RawtypeInstantiation<C>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you think of a better class name? The key isn't rawness, it's the C extends R<C>
bound, right?
|
||
public class RawtypeInstantiation<C extends RawtypeInstantiation<C>> { | ||
|
||
void foo() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you pick better names than foo
and bar
?
RawtypeInstantiation rawtypeInstance = new @A RawtypeInstantiation(); | ||
// :: warning: (cast.unsafe.constructor.invocation) | ||
RawtypeInstantiation<D> genericInstance = new @A RawtypeInstantiation<>(); | ||
// :: warning: (cast.unsafe.constructor.invocation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these warnings distract from the point of this test. Can you add a suitable constructor and suppress the warning there once?
// :: warning: (cast.unsafe.constructor.invocation) | ||
RawtypeInstantiation<D> genericInstance = new @A RawtypeInstantiation<>(); | ||
// :: warning: (cast.unsafe.constructor.invocation) | ||
RawtypeInstantiation<?> wildcardInstance = new @A RawtypeInstantiation<D>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do these tests ensure the proper viewpoint adaptation happened?
if (visitedTypes.contains(declared)) { | ||
return declared; | ||
} | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there really exceptions that can happen in this code? Adding these large try blocks makes it hard to see what changed.
Fixes #778