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

Make visitors able to visit any domain type #66

Open
5 tasks
dlurton opened this issue Jan 18, 2021 · 4 comments
Open
5 tasks

Make visitors able to visit any domain type #66

dlurton opened this issue Jan 18, 2021 · 4 comments

Comments

@dlurton
Copy link
Member

dlurton commented Jan 18, 2021

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) from PermuteTransformTests
@RCHowell
Copy link
Member

RCHowell commented Jun 30, 2022

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)

@RCHowell
Copy link
Member

If visitors have parameterized return types like discussed here -- #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

@RCHowell
Copy link
Member

RCHowell commented Jun 30, 2022

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?

@RCHowell
Copy link
Member

RCHowell commented Jun 30, 2022

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

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

No branches or pull requests

2 participants