Consider hiding the distinction between Select & Ident

[DarkDimius]

In Dotty, Select & Idents are very interchangeable and the only reliable way to see if this is a selection or a direct reference is to look at the type.

Correct handling of them is counterintuitive, as one would assume he could trust the tree, but he cant: https://github.com/lampepfl/dotty/blob/master/src/dotty/tools/backend/jvm/DottyBackendInterface.scala#L428-L438

This introduces misconceptions such as the one happened at #1685

Note that we already have a tree type that should be used instead: RefTree.

[liufengyun]

@DarkDimius you mean we add a transform to convert Ident to Select according to its type? I think that's a good idea.

If you mean replace Select and Ident by RefTree in AST, that seems problematic to me. From the parser's point of view, trees are used to represent syntactical information, we need to differentiate them.

[DarkDimius]

@DarkDimius you mean we add a transform to convert Ident to Select according to its type? I think that's a good idea.

as new trees can be introduced by any phase after the transform would break this inveriant, it's not what I propose.

If you mean replace Select and Ident by RefTree in AST, that seems problematic to me. From the parser's point of view,

Neither do I propose to change parser. The current situation is that distinction between Ident and Select is only syntactic, but not semantic.

Eg given a source:

package bar;

class Foo {
   val s = 1
}

// somewhere in method
val f = new Foo
import f._
f.s
s

Ident(Foo) and Select(bar, Foo) as well as Ident(s) and Select(f, s) have the same semantics and should be transformed uniformly. The current pipeline after typer sees a lot of distinction between the two and needs to handle them separately, introducing place for duplication and inconsistencies.

@liufengyun, I propose to consider options below:

• replace transformIdent and transformSelect with transformRef
• revamp internal semantic api to ensure that it does not favor knowing if a tree in hand is an Ident or a Select.

NB:

@DarkDimius you mean we add a transform to convert Ident to Select according to its type? I think that's a good idea.

this might also be a bad idea from performance point of view. Idents and Selects are very common node types. If anything, I'd actually prefer to convert as many Selects to Idents as possible to reduce the footprint of the tree.

[odersky]

I think the problem is that if the type of a qualifier is a path, we have a tendency to leave it as an Ident. And this is indeed important for performance. Otherwise, think about all the Selects we would create when we select a global symbol in some object in some nested package...

But if the type if a ref is not a path then we lose information. E.g. if e is an unstable expression of type C then the Select e.m would have type C#m. That type loses information so we need to keep the Select around.

Not sure what's a good way to generalize this, and which does not involve converting almost everything to a Select.

[Blaisorblade]

Otherwise, think about all the Selects we would create when we select a global symbol in some object in some nested package...

That is, if y means a.b.c. ... .x.y, you don't want to create nested Select nodes, so produce i: Ident and encode the implicit path in i.tpe: Type? What about having instead one Select(PathTerm(tpe: Type), y) instead, where PathTerm(tpe) wraps the Type and tpe is the same as i.tpe? Details are probably questionable, but this avoids creating an isomorphic copy of a path while still being handled correctly.

If you don't want even one wrapper, what about distinguishing (after the appropriate phase) Ident that must be treated specially from ones that don't with a different type of Ident? (Naively, I'd think you want to distinguish "local variables" from fields but I'm sure there's more). After all, to a first approximation those are more "unrelated constructs" than "the same construct", whatever the concrete syntax might suggest.

as new trees can be introduced by any phase after the transform would break this inveriant, it's not what I propose.

Couldn't the invariant be tested and enforced, and the later phases be adapted? Of course changing all the existing references is lots of work.