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

Reword callable reference and class literal resolution algorithm, explain some corner cases (#5) #43

Merged
merged 7 commits into from
Aug 15, 2016

Conversation

udalov
Copy link
Member

@udalov udalov commented Aug 10, 2016

No description provided.

fun test() {
C::class // class of C
C()::class // class of C
(C)::foo // class of C.Companion
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be ::class?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, thanks

2. Resolve the unbound reference with the existing algorithm.
1. Type-check the LHS as an **expression**.
- If the result represents a _companion object of some class_ specified with the short syntax (via the short/qualified name of the containing class), discard the result and continue to p.2.
- If the result represents an _object_ (either non-companion, or companion specified with the full syntax: `org.foo.Bar.Companion` or just `Bar.Companion`), remember the result and continue to p.2.
Copy link
Contributor

Choose a reason for hiding this comment

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

or (Bar)

Copy link
Member Author

Choose a reason for hiding this comment

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

No, (Bar) is covered by the first clause because it's a companion object of some class specified with the short syntax

Copy link
Contributor

Choose a reason for hiding this comment

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

What about object Foo (Foo)?

Copy link
Member Author

@udalov udalov Aug 12, 2016

Choose a reason for hiding this comment

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

I'm not sure what do you propose, could you elaborate? The two examples here explain what does "companion specified with the full syntax" mean, they do not relate to non-companion objects. I don't think usage of a non-companion object needs any explanation. Or are you talking about the (Foo)::class case?

UPD: discussed in person

@abreslav
Copy link
Contributor

No other comments from me

@udalov
Copy link
Member Author

udalov commented Aug 12, 2016

@abreslav @erokhins I've updated the class literal type checking part with a more precise definition of what happens with expressions of type kotlin.Array<...>. Please review, I'm not sure I've formulated it well enough

@@ -162,12 +162,24 @@ fun test() {
}
```

Once the LHS is type-checked and its type is determined to be `T`, the type of the whole class literal expression is `kotlin.reflect.KClass<T'>` where `T'` is a type obtained by _substituting `T`'s arguments with star projections (`*`)_. Example:
Once the LHS is type-checked and its type is determined to be `T`, the type of the whole class literal expression is `KClass<erase(T)>`, where `erase(T)` represents the most accurate possible type-safe representation of the type `T` and is obtained by substituting `T`'s arguments with star projections (`*`), except when the type is an array.
Copy link
Contributor

Choose a reason for hiding this comment

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

Type-arguments of arrays are not fully known at runtime either: their nullability is unclear. Does it matter in this context?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does matter, and we may revisit it in the future, but type-checking arrayOfStrings::class to KClass<Array<*>> would be rather unhelpful from Java user's point of view. We've briefly discussed this with @erokhins and decided (for now) to type-check arrayOfStrings::class to KClass<Array<String>>. We should discuss this in detail later anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Note in JS we don't know anything about type-arguments of arrays at runtime.

@udalov
Copy link
Member Author

udalov commented Aug 15, 2016

Added a TODO about JS, we'll have to discuss later. I'm merging this set of changes now, thanks to everyone for review!

@udalov udalov merged commit 9eed7b5 into master Aug 15, 2016
@udalov udalov deleted the udalov branch August 15, 2016 15:35
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.

5 participants