Origin and Destination

[ftomassetti]

I was writing some documentation on this subject (https://github.com/Strumenta/StarLasu/blob/main/documentation/origin_and_destination.md), and I got a few doubts.

Currently, a Node in Kolasu has a destination specified like this:

var destination: Position? = null

• Should the destination of a Node be something more generic that a Position? The destination could be another node. For example, if a I generate Java from RPG, each RPG Node could have as destination a Java Node
• Should the destination and the Origin be potentially many? For example, I could generate multiple things from a single AST (so multiple destinations) and I could generate a Node using information dispersed over multiple Nodes (so multiple origins). In this case, maybe it would be convenient to create a CompositeOrigin and CompositeDestination, instead of having destination become destinations and have type list and origin become origins and have type list also
• Should destination and origin actually have the same type? Maybe "Location"?

[alessiostalla]

These are very valid points. I like CompositeOrigin and CompositeDestination also because they enable us to support new strategies in the future (e.g. save the last n code generation results) without having to modify Node or provide specialized subclasses of Node.

I'm unsure about a shared type because some objects only make sense as origins (e.g. a parse tree) and others only make sense as destinations (e.g. generated code). Also, a Node can be at the same time both an origin and a destination.

[ftomassetti]

Another doubt related to this:
Should positionOverride move from Node to Origin?

The reason would be that the Node per-se contains the information, where the information comes from is defined by the Origin, and so the position and the information to determine it seems to belong to Origin

[alessiostalla]

Regarding non-text positions, I think it's a different concern, but I would probably model that with a specialized Origin subclass that has a null position

[ftomassetti]

I think API ergonomics is very important: we do not want to sacrifice the ergonomics of the most common cases to support overly generic cases.

What if we preserve the position setter in Node and make it forward the change to Origin? If Origin is not set, we could either throw an error or create some form of default origin.

I did an experiment with that here Strumenta/kolasu@bf135b6

The external API of Node would not change:

    @property:Internal
    override var position: Position?
        get() = origin?.position
        set(position) {
            if (origin == null) {
                origin = UnknownOrigin(position)
            } else {
                origin!!.position = position
            }
        }

It is not super-clean, but it is backward compatible. Some Origins may also throw an exception if a position does not make sense for them

[alessiostalla]

What happens though if one sets the position first and the origin later? The new origin would have to be mutated to preserve the manually set position... I don't know, it looks overly complicated to me and I don't understand the benefits, but I don't want to sound too negative...

[ftomassetti]

Conceptually I think that positionOverride is a characteristic of the Origin and not the Node per se, because the Node contains the information, not where the information comes from. The Node could have been instantiated in memory, being obtained from code generation, or represent a Node in a projectional editor. positionOverride is a correction on the parseTreeNode, typically due to the fact that we executed ANTLR on a subset of the input and not the whole input, so the line and column are wrong.

However, it is true that 90% of what we do involves parsing, so having the possibility to at the very least read the Position directly from the Node is useful. The alternative would be accessing the origin, maybe subcasting it and then access the Position. We would end up sacrificing convenience for supporting the general case.

Maybe we can combine the two by making possible to set positionOverride only when ParseTreeOrigin is instantiated. This may cause the need for refactoring of some existing code.

Perhaps we can create an issue to revisit that in Kolasu 1.6 and leave as it is for now, as the consequences would be not small.

[alessiostalla]

Thinking better about this, I remember the design originally was similar. However, this created complexity in Node because then you have to handle setting the position first and the origin later and vice versa, and it jeopardized the objective of making Node unaware of any Position logic. Also, having the position override in Node made it easier to update from Kolasu 1.4 where each Node had an override var specifiedPosition.

However we could move positionOverride back to Origin (and restore the JustPosition subclass of Origin) without complicating Node too much if we also moved the origin merging logic from Node to Origin, so that node.origin = newOrigin is not a simple set but a merge operation (where JustPosition would set the position override on the merged origin).

This still looks too complicated to me for what it's worth but it could be a good compromise.