-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
KE2: Start working on KtTypes #18031
base: ke2
Are you sure you want to change the base?
Conversation
val classId = addClassLabel(c.symbol as KaClassSymbol) | ||
val javaResult = TypeResult(classId /* , TODO, TODO */) | ||
val kotlinTypeId = | ||
tw.getLabelFor<DbKt_class_type>("@\"kt_class{$classId}\"") { |
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.
Minor: We typically have a ;
separator between parts of the label. So I think this should be "@\"kt_class;{$classId}\""
val kotlinResult = TypeResult(fakeKotlinType() /* , "TODO", "TODO" */) | ||
val classId = addClassLabel(c.symbol as KaClassSymbol) | ||
val javaResult = TypeResult(classId /* , TODO, TODO */) | ||
val kotlinTypeId = |
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.
Is this correct? Shouldn't we check the nullability of the KaClassType
?
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.
Ah, the nullability belongs in useType
, but it looks like that I got so focused on working out what the dbscheme tables should look like that I didn't add it. I'll add a commit for that.
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.
Done. I'm not sure it's perfect yet, and we'll have to think about how to represent the definition of a type alias, but I think this gives us something to work with in the mean time.
This adds the beginning of KtType extraction.
The main change is in the dbscheme relations, which now look like:
and
useType
now produces these.This broke the KE1 build, which for now I've just fixed with
TODO()
calls; I think it makes sense to fix this properly later, once we are confident that this is the right way to go.fakeKotlinType
is now gone; we should make proper Kotlin types in cases where we used to use it.