Lifter revamp

[CAG2Mark]

See #369

[LPTK]

Oh, I didn't realize the handler refactor PR was made against the remove-scope-hacks branch. We should have changed that before the merge.

By the way, your rebased commit does not compile.

[CAG2Mark]

Remaining tasks:

• Add back handler paths (so we are allowed to lift classes that extend handler-related classes)
• Lift objects

[CAG2Mark]

Seems that a lot of the complexity from before came from dealing with nested modules and objects...

Things were pretty clean until I added nested modules and objects.

[CAG2Mark]

OK so a new pain point: I currently lift classes by putting the extra variables in private fields. But since classes could be instantiated immutably, we are forced to put all variables they mutate into captures. Any idea on what to do? Or should we just give up and put them into captures?

EDIT: apparently private fields remain mutable after freezing, which also fixes the other problem I was worrying about (that I never mentioned)

[AnsonYeung]

I still need some time to read through rest of the code. I'll review the rest later.

[AnsonYeung]

I can't really look through every single line of the code. The main problem I have about the current state is that I found some code that is unreachable (never tested). If those branch is truly unreachable they should just crash instead of returning something that may not be correct. There is also some instance of dead variable and I wonder if there is some lint option to find them all and remove them.

[CAG2Mark]

Thanks, I'll address these tomorrow.

[LPTK]

(For the record, Anson did not work on this or the previous lifter)

I know, but Anson is very familiar with what the lifter does and the IR the lifter works on. Also, I can't review the PR at the moment since I am on a family vacation.

[CAG2Mark]

OK, I realized there's a few tricky cases with objects and modules that I need to sort out again...

[CAG2Mark]

Note that this is not yet ready to merge. I still need to deal with some problems with objects.

Honestly, I might just rewrite all objects before lifting because that will make things 100x easier

[CAG2Mark]

OK, objects are causing a lot of problems. There are a few options I have right now:

• Don't lift objects for now. I actually do not think having objects in the IR is a good idea, since it is trivial to create singleton classes, and we can express that they are singleton by specifying they are objects at the definition site.
• Properly support objects in the lifter, which will make it even more complicated and annoying. The main problem right now is cases like this :

  object M with
    object MM
  M.MM

  I have to make a new public field for MM. But to properly rewrite the selection M.MM, I need to add a lot of ad-hoc stuff just to support this case.
• Write another pass that rewrites some objects as classes. Then everything work properly, but it feels a bit unclean. I actually already have such a class written, but I haven't pushed my changes yet since there's still a few small problems.

Not sure what the best option is though...

[LPTK]

I actually do not think having objects in the IR is a good idea, since it is trivial to create singleton classes,

But the semantics is different. As discussed before, what makes object definitions useful is their initialization semantics (they allow for indirect self-reference).

fun f = print(A)
object A with
  f // A is not fully initialized yet but can still be accessed
  val x = 1
//│ > A {}

// By contrast:
fun f = print(A)
class Acls with
  f // A is not accessible at all here
  val x = 1
val A = new Acls
//│ > undefined
//│ A = Acls { x: 1 }

// We still need an initialization checker to catch things like this:
fun f = print(A.x)
object A with
  f
  val x = 1
//│ > undefined

That said, it is probably (?) not a crucial feature.

and we can express that they are singleton by specifying they are objects at the definition site.

Could you elaborate what you mean by that?

• Properly support objects in the lifter, which will make it even more complicated and annoying. The main problem right now is cases like this :

  object M with
    object MM
  M.MM

  I have to make a new public field for MM. But to properly rewrite the selection M.MM, I need to add a lot of ad-hoc stuff just to support this case.

Do note that this is pretty much the same situation as supporting classes in classes, which we intentionally do not support in the lifter for now (it will be possible to properly support once we resolve all selections statically).

• Write another pass that rewrites some objects as classes. Then everything work properly

Except it doesn't, as it changes the initialization behavior.

So, either we change the initialization semantics uniformly (making top-level objects initialize the same as nested ones after lifting, ie the same as simple classes + val members), or we support the existing one properly in the lifter. The former seems much easier and is probably the way to go for now.

For this PR, though, I suggest omitting object support. We should add that support in a follow-up PR.

[LPTK]

Also, if we go with the former approach, then there's no real need to have an object keyword and a corresponding IR form at all.

[CAG2Mark]

Could you elaborate what you mean by that?

OK, I previously thought the "different initialization semantics" was just that it's a singleton object and we could use a flag or something at the definition site to denote that. I didn't realize they had the semantics you mentioned.

[CAG2Mark]

BTW it should be ready

[LPTK]

As mentioned before, I can't really review the PR in depth for now. But it's been open for long enough, so I think we should merge it. And we'll discuss the follow up work on objects/modules later in person.

[LPTK]

BTW it should be ready

Don't forget that we need to resolve the open conversations.

[CAG2Mark]

OK, everything is addressed