-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Ycheck that all methods have method type #1205
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
Conversation
Why does this fail? |
Some tests failed related to the tree checking in this commit. I'll investigate. Most likely I missed some valid types here. |
@@ -237,6 +237,14 @@ class TreeChecker extends Phase with SymTransformer { | |||
tree | |||
} | |||
|
|||
/** Check that all methods have either MethodicType or PolyType(MethodicType) */ | |||
def isMethodType(pt: Type)(implicit ctx: Context): Boolean = pt match { |
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.
Failing tests suggest that you may be missing AnnotatedTypes here.
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 @DarkDimius , AnnotatedType
is indeed missing here. But there's something more related to default parameters.
Following generated function for default parameters
def copy$default$1[a]: a @uncheckedVariance =
Some.this.x: a @uncheckedVariance
gets following type:
PolyType(List(a), List(TypeBounds(TypeRef(ThisType(TypeRef(NoPrefix,scala)),Nothing), TypeRef(ThisType(TypeRef(NoPrefix,scala)),Any))), AnnotatedType(TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,<empty>)),Some)),Some$$a),ConcreteAnnotation(Apply(Select(New(TypeTree[TypeRef(ThisType(TypeRef(NoPrefix,unchecked)),uncheckedVariance)]),<init>),List()))))
Is it valid for methods of default parameters to take a non-method type?
f11e850
to
60a0438
Compare
60a0438
to
cf50b72
Compare
retest this please |
cf50b72
to
371e8a7
Compare
LGTM |
Fix #1046 . Please review @DarkDimius .