partiql-ir-generator
partiql-ir-generator copied to clipboard
Make visitors able to visit any domain type
Given a simple domain:
(define (domain simple (product foo))
For the Kotlin target we get something like:
class Simple {
...
abstract class SimpleNode { ... }
...
}
However, none of the generated visitors can accept instances of SimpleNode
. This is a problem if you have an instance of SimpleNode
and want to send it thru one of the visitors. The user has to write a when
statement and dispatch to the appropriate visitor method. PIG's own tests (within PermuteTransformTests
) for example contain the following code:
// https://github.com/partiql/partiql-ir-generator/issues/66
val actual = when(tc.input) {
is TestPermuteDomainA.ProductA -> tt.transformProductA(tc.input)
is TestPermuteDomainA.RecordA -> tt.transformRecordA(tc.input)
is TestPermuteDomainA.SumB -> tt.transformSumB(tc.input)
// `else` is needed since TestPermuteDomainA.TestPermuteDomainANode is not currently a sealed class
else -> fail("unexpected type")
}
This is potentially a pain point for users and should really be part of the generated visitors:
class Simple {
open class Visitor : DomainVisitorBase() {
...
// Add this:
fun walkNode(node: SimpleNode) { T.B.D. }
...
}
open class VisitorFold<T> : DomainVisitorFoldBase<T>() {
...
// Add this:
open protected fun walkNode(node: SimpleNode, accumulator: T): T = T.B.D.
// Add this:
open fun walkNode(node: SimpleNode, accumulator: T) = T.B.D.
...
}
abstract class VisitorTransform : DomainVisitorTransformBase() {
...
// Add this:
open fun transformNode(node: SimpleNode): SimpleNode = T.B.D.
...
}
}
As part of this we should consider if it is possible and advisable to make SimpleNode
a sealed
class.
- [ ] Evaluate if it is feasible and wise to make the generated domain node super class (i.e.
SimpleNode
)sealed
, and if so, make it so. - [ ] Add the noted function for in generated
DomainVisitorBase
implementations - [ ] Add the noted functions
DomainVisitorFoldBase<T>
implementations - [ ] Add the noted function
DomainVisitorTransformBase
implementations - [ ] Remove the (now) redundant
when
block (shown in the example above) fromPermuteTransformTests
I'm curious how this is different than using accept? I believe the intention of an accept
method is to invoke the appropriate visitor method when then concrete type in unknown. A "reverse" accept like the when
clause you are using is useful when you cannot modify or do not have an accept
method of nodes. Considering we generate the code, we should be generating the appropriate accept
methods.
val actual = tc.input.accept(tt)
If visitors have parameterized return types like discussed here -- https://github.com/partiql/partiql-ir-generator/issues/123 then you include the parameterized return type and context objects in the accept. The current state of visitors/transformers/etc. is all confusing and should be consolidated to a single abstraction to avoid this confusion
Here's a modified example from an old project
interface IRVisitor<R, C> {
fun visit(node: IRNode, ctx: C): R? = defaultVisit(node, ctx)
// "reverse" visit for nodes A, B extending X
// you are proposing generating one of these for each sum type and the whole domain
fun visit(node: IRNodeX, ctx: C): R? = when (node) {
is IRNodeA -> visit(node, ctx)
is IRNodeB -> visit(node, ctx)
}
// .... removed
}
sealed interface IRNode {
override fun <R, C> accept(visitor: IRVisitor<R, C>, ctx: C): R? = visitor.visit(this, ctx)
}
Then given some IRVisitor and an arbitrary IRNode, we can call accept on the IRNode to invoke the appropriate visitor method via function overloading.
Am I missing something here?
You call ctx
accumulator, but the visit doesn't have to be a fold. Visitor ctx like this is useful for passing arbitrary context (which may be an accumulator) like indentation level for pretty printing for example