Refactor TransformerOps into a helper to operate on state in Context

[Plixo2]

I want to remove the Cake Pattern from the Context class, starting with TransformerOps

In this PR i only did 3 simple things:

1. Moved the bindings list into a separate BindingDB class.
   (1 new file, 1 line modified)
2. Converted the TransformerOps into a helper that operates on the BindingDB.
   (~8 lines modified)
3. Replaced the corresponding calls of Context.F(..) with TransformerOps.F(...)
   (~27 single words modified)

This paves the way to refactor all the remaining 10 traits and to improve handling of state later on, which should improve the onboarding process of new contributors.

I have many arguments on why this refactoring should be done, but just consider this:

There are 245 methods accessible through the Context class and 18 of them are just called bind with different signatures

[b-studios]

Disclaimer: I won't have a lot of time to review the next week, but here is a remark about how to go about it:

A lot of phases, other than the frontend, use phase-specific contexts that are defined in the same file and passed around via implicits. Maybe one way of getting rid of the cake pattern is to do this for one file in the frontend-phase after another.

Before we do this, we need to figure out

1. whether we want to name those contexts uniformly
2. what to do with error reporting
3. what to do with the annotations db

Maybe you could start by looking at the other phases and summarize how we to it there? This would help to identify a single pattern that we can then apply.

What do you think?

[Plixo2]

Thanks for the feedback.

Start by looking at the other phases and summarize how we do it there?
[...] Identify a single pattern that we can then apply

I already refactored NamerOps, TyperOps and Timers using the same pattern as TransformerOps:

• Define a new [xxx]DB class
• Add this class as a field in the Context class
• Change [xxx]Ops into a object that operates on the DB.

I also took a look at Unification, SymbolAnnotations and the other annotation DB's. They would follow the same pattern.

ErrorReporter can stay as a trait on the Context for convenience (for now).
Writing panic instead of ErrorReporterOps.panic is definitely better. Also Errors are not coupled to the 'state' of the compiler per se

Maybe one way of getting rid of the cake pattern is to do this for one file in the frontend-phase after another

The changes i suggest for now would only focus on the main Context class and would require minimal review time and refactoring. Creating separate Contexts for each Phase would require deeper changes (resulting in more review time) and probably need some sort of separation of DB's i propose anyway in some form or another.

When you, and your team, have more time on you hands, i would suggest tightly controlling the context each stage requires and produces:

Details

Right now the Phases are defined like this:

enum PhaseResult {
  /**
   * The result of [[Namer]] resolving all names in a given syntax tree. The resolved symbols are
   * annotated in the [[Context]] using [[effekt.context.Annotations]].
   */
   case NameResolved(source: Source, tree: ModuleDecl, mod: symbols.Module)
   
   
    // etc
}

Why not just move all the DB's into the phase result?, so like this:

case NameResolved(source: Source, tree: ModuleDecl, mod: symbols.Module, scopeDB: ScopeDP, errorDB: ErrorDB, etc..)

And then just create a new class to operate on these DB's as the phase-specific context and then just pass the new context out as part of the phase result...

This would clearly show and control what each phase needs and produces. This change would exactly start of where my suggested changes end.

Maybe i dont see the greater picture here or just dont get something else entirely.

[b-studios]

Maybe I didn't make myself clear. When I said

A lot of phases, other than the frontend, use phase-specific contexts that are defined in the same file and passed around via implicits.

I meant something like

effekt/effekt/shared/src/main/scala/effekt/core/optimizer/Normalizer.scala

Line 33 in 1f13568

case class Context(

or

effekt/effekt/shared/src/main/scala/effekt/machine/Transformer.scala

Line 551 in 1f13568

class BlocksParamsContext() {

We have at many different compiler passes and many of them need to maintain contextual information. I purposefully said "other than the frontend", since the frontend (lexer, parser, namer, typer, etc.) is the only part where we use the cake pattern context.

So gathering how

• how the context is represented,
• how we name things,
• how we pass it along,
• whether the context has methods or they are represented as functions in the same file (taking the context object)
• etc.

could be helpful since it would inform us in our decisions.

[Plixo2]

I am sorry that I missed your point

I will take a look tomorrow.

[timsueberkrueb]

So gathering how

how the context is represented,
how we name things,
how we pass it along,
whether the context has methods or they are represented as functions in the same file (taking the context object)
etc.

could be helpful since it would inform us in our decisions.

I think it makes sense to collect this in a GitHub discussion.

[timsueberkrueb]

Regarding this PR: I think we need some target design that describes where we want to end up with this, otherwise it's hard to judge if the changes in this PR move us towards this target. I think looking at the patterns in the compiler backend and drafting a new design for separate disentangled contexts for the frontend would be a good starting point.

[timsueberkrueb]

If I would dream up a design, I would suggest separate contexts for all phases, e.g. NamerContext, TyperContext, etc. and moving all the state and functionality out of the various *Ops into the phase-specific contexts. At the moment, it is not immediately clear to me how this PR would help reach this goal, as it separates out the state from TransformerOps to Context. However, I might be wrong about this and it is hard to tell without a target design! :)

My current intuition would be to start by splitting up Context and implement the relevant Ops for the split-up contexts. For example, if we'd start with namer, we might do something like the following:

• PR 1: Create a NamerContext that inherits Context, then just change enough code to pass the new NamerContext through Namer and don't change anything else
• PR 2: Implement NamerOps for NamerContext and remove it from Context
• PR 3: Move all logic and state that is relevant to Namer to NamerContext and create the "big" Context after Namer and pass it to the rest of the frontend
• Repeat for the next phase

Big disclaimer: This is not meant as a concrete plan for you to follow (perhaps it doesn't even make sense); it is just to give you an idea on what my current mental model looks like. I would advocate for doing the following:

• Research how the frontend Context is currently setup (you've done that already) and post a summary in a GitHub discussion
• Research how contexts are managed in the backend and how they differ from frontend and post a summary in a GitHub discussion (suggested by @b-studios)
• Propose a target design in a GitHub discussion based on the findings. Once we agree with the target design, we build a rough plan on how we realize the refactoring in reasonably sized steps