eclipse-archived / ceylon

The Ceylon compiler, language module, and command line tools
http://ceylon-lang.org
Apache License 2.0
399 stars 62 forks source link

Compiler loses information about order of CaseTypes #4053

Open CeylonMigrationBot opened 10 years ago

CeylonMigrationBot commented 10 years ago

[@lucaswerkmeister] The following caseTypes

class T of a|A|X|X.Y|A.B|B|B.B|x|y|Z|z|Y { }

are parsed into the following AST:

+ of [CaseTypes] (1:8-1:39)
|  +  [BaseType] (1:13-1:13)
|  |  + A [Identifier] (1:13-1:13)
|  +  [BaseType] (1:15-1:15)
|  |  + X [Identifier] (1:15-1:15)
|  + . [QualifiedType] (1:17-1:19)
|  |  + Y [Identifier] (1:19-1:19)
|  |  +  [BaseType] (1:17-1:17)
|  |  |  + X [Identifier] (1:17-1:17)
|  + . [QualifiedType] (1:21-1:23)
|  |  + B [Identifier] (1:23-1:23)
|  |  +  [BaseType] (1:21-1:21)
|  |  |  + A [Identifier] (1:21-1:21)
|  +  [BaseType] (1:25-1:25)
|  |  + B [Identifier] (1:25-1:25)
|  + . [QualifiedType] (1:27-1:29)
|  |  + B [Identifier] (1:29-1:29)
|  |  +  [BaseType] (1:27-1:27)
|  |  |  + B [Identifier] (1:27-1:27)
|  +  [BaseType] (1:35-1:35)
|  |  + Z [Identifier] (1:35-1:35)
|  +  [BaseType] (1:39-1:39)
|  |  + Y [Identifier] (1:39-1:39)
|  +  [BaseMemberExpression] (1:11-1:11)
|  |  + a [Identifier] (1:11-1:11)
|  |  +  [InferredTypeArguments]
|  +  [BaseMemberExpression] (1:31-1:31)
|  |  + x [Identifier] (1:31-1:31)
|  |  +  [InferredTypeArguments]
|  +  [BaseMemberExpression] (1:33-1:33)
|  |  + y [Identifier] (1:33-1:33)
|  |  +  [InferredTypeArguments]
|  +  [BaseMemberExpression] (1:37-1:37)
|  |  + z [Identifier] (1:37-1:37)
|  |  +  [InferredTypeArguments]

As you can see, all the BaseMemberExpressions – the lowercase, or object, cases – now come after the “real” (uppercase) types, because the parser adds them to a separate list. This doesn’t matter for the compiler itself, because the order of case types is irrelevant, but it’s very critical for tools that operate on the source-code level (for example, a formatter).

The simplest way to fix this – without restructuring the class hierarchy – would be to add a List<Node> to CaseTypes that contains BaseMemberExpressions and StaticTypes. (Of course, in Ceylon, you could make it a List<StaticType|BaseMemberExpression> and remove the original lists. Aren’t union types awesome?)

[Migrated from ceylon/ceylon-spec#947]

CeylonMigrationBot commented 10 years ago

[@lucaswerkmeister] You could also construct that list at runtime like this (pseudo-ish code):

List<Node> getCases() {
    List<Node> ret = new ArrayList<>();
    ret.addAll(getTypes());
    ret.addAll(getBaseMemberExpressions());
    ret.sort(byAscending(Node.getLocation));
    return ret;
}
CeylonMigrationBot commented 10 years ago

[@lucaswerkmeister] Workaround (Ceylon):

CaseTypes that = ...;
MutableList<Node> casesList = ArrayList<Node>();
casesList.addAll(CeylonIterable(that.types));
casesList.addAll(CeylonIterable(that.baseMemberExpressions));
assert (nonempty cases = casesList.sort(byIncreasing(compose(Token.tokenIndex, Node.token))));

Note that it would still be better to properly record this information instead of extracting it from the tokens like this – this workaround breaks if the AST wasn’t parsed from a token stream.

CeylonMigrationBot commented 10 years ago

[@lucaswerkmeister] Also note that in the workaround, casesList should really be a MutableList<StaticType|BaseMemberExpression>, but #1572 currently prevents this.