-
Notifications
You must be signed in to change notification settings - Fork 66
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
[Compiler plugin] Lower frontend generated implicit receivers #869
Conversation
@@ -11,6 +12,7 @@ import org.jetbrains.kotlinx.dataframe.impl.columnName | |||
import kotlin.reflect.KProperty | |||
import kotlin.reflect.typeOf | |||
|
|||
@HasSchema(schemaArg = 0) |
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.
What does HasSchema do again? There's no docs :/
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.
To help identify type parameter that represents dataframe schema
org.jetbrains.kotlinx.dataframe.plugin.InterpretKt#getSchema
org/jetbrains/kotlinx/dataframe/plugin/interpret.kt:255
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.
right, so if we would take the type parameter at schemaArg
, which in AggregateDsl<out T>
is out T
, and put it in DataFrame<>
like DataFrame<T>
, a schema generated for thát would represent this object. And this could be different for types like GroupedDataRow<out T, out G>
where G
might represent the data schema?
Could you explain this in a small piece of KDoc?
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.
I'll think of something
fun box(): String { | ||
val df = List(10) { Record(it.toString(), it) }.let { dataFrameOf(*it.toTypedArray()) } | ||
val aggregate = df.pivot { b }.aggregate { | ||
this.add("c") { 123 }.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.
which part failed before and is now solved?
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.
execution of this expression this.add("c") { 123 }.c
failed due to wrong receiver in generated bytecode. c
extension property used this@aggregate
instead of scope it comes from
@OptIn(UnsafeDuringIrConstructionAPI::class) | ||
private fun IrExpression.replaceWithConstructorCall(): IrConstructorCallImpl { | ||
val constructor = type.getClass()!!.constructors.toList().single() | ||
val type = type |
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.
unnecessary assignment
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!
47d623c
to
358fcd9
Compare
Generated sources will be updated after merging this PR. |
For extension properties to work, frontend plugin inserts implicit receivers. But they are just non-existing references needed for resolve. Backend must find them and replace with something that will be executed