bnorm/kotlin-power-assert

Compiler crash when trying to extend the plugin to kotlin.require and kotlin.check

mikehearn opened this issue · 5 comments

mikehearn commented

Having power assertions on assert() is nice, but almost all the assertions in my codebase are require or check. So:

configure<com.bnorm.power.PowerAssertGradleExtension> {
    functions = listOf("kotlin.assert", "kotlin.test.assertTrue", "kotlin.require")
}

But, with Kotlin 1.5 this doesn't work, kotlinc crashes with the attached crash dump.

dump.txt

bnorm commented

I don't think this is specific to require as it is not fundamentally different from assert; I think this more has to do with a complicated require boolean expression that is causing the plugin to generate bad code. Would you be able to share a code snippet from this project with me?

From the IR dump I see in the function hydraulic.utils.config.ConfigReaderTest.propertyAsserts there is a call to hydraulic.utils.config.ConfigReaderKt.reqAndAlso of which the check parameter contains a require call which I'm curious about. Share as much as you are comfortable with but hopefully this will help recreate a failing unit test I can work against.

bnorm commented

Well I said complicated, but turns out it doesn't need to be complicated:

val list = listOf("Jane", "John")
assert(list.any { it == "Bob" })

The technically details are that it is trying to pull things out of the lambda body as temporary variables and that just breaks the whole thing because it doesn't exist outside the lambda. Basically it is trying to generate this:

val list = listOf("Jane", "John")
val tmp0 = list
val tmp1 = it // from lambda
val tmp2 = tmp1 == "Bob" 
val tmp3 = return tmp2 // technically an expression...
val tmp4 = { tmp3 }
val tmp5 = tmp0.any(tmp4)
assert(tmp5, ... diagram with tmp variables ...)

I think realistically I can keep tmp0 and tmp5 but everything else can't really be diagrammed unless someone has a clever idea.

val list = listOf("Jane", "John")
val tmp0 = list
val tmp5 = tmp0.any { it == "Bob" }
assert(tmp5, ... diagram with tmp variables ...)
mikehearn commented

I think it's reasonable to just skip any assertions with lambdas in them, yeah.

bnorm commented

Fixed in version 0.8.1.