ceylon / ceylon.ast

Apache License 2.0
18 stars 3 forks source link

Add support for `of package.foo` #134

Open lucaswerkmeister opened 7 years ago

lucaswerkmeister commented 7 years ago

See ceylon/ceylon#4368 and ceylon/ceylon@7a102d2b45871ebf8dadc7655d69fa697e2a19ab.

lucaswerkmeister commented 7 years ago

Ouch, this even broke the build…

lucaswerkmeister commented 7 years ago

I feel like a better way to implement package.stuff would be to remove the Package node as a separate Expression, and instead nest it inside a BaseExpression (either as a flag, or as an optional, contained SelfReference). package.foo would be a BaseExpression instead of a QualifiedExpression, which makes it automatically legal in case types and similar situations. WDYT @gavinking @jvasileff?

gavinking commented 7 years ago

Sure, I agree, that's better. It's what I do for types.

jvasileff commented 7 years ago

I suspect that would be slightly better from the perspective of writing a backend. What I have now is this pattern in a couple places:

if (that.receiverExpression is Package) {
    // treat as BaseExpression using that.nameAndArgs
}
lucaswerkmeister commented 7 years ago

Oh, I didn’t even realize that package isn’t even a SelfReference per the spec, so changing this wouldn’t even depart from the spec that much. I guess it would just be removing Package altogether and instead sticking a PackageQualifier in a BaseExpression.

jvasileff commented 7 years ago

+1 for this feature!

Compilation currently fails with:

source/ceylon/ast/redhat/CaseTypes.ceylon:46: error: argument must be assignable to parameter 'collecting' of 'collect' in '[Tree.JStaticType|Tree.StaticMemberOrTypeExpression+]': '<PrimaryType|LIdentifier>(JStaticType|JBaseMemberExpression)' is not assignable to '<PrimaryType|LIdentifier>(Tree.JStaticType|Tree.StaticMemberOrTypeExpression)'
    value result = CaseTypes(cases.collect(propagateUpdate(primaryTypeOrMemberNameToCeylon, update)));
                                           ^
jvasileff commented 7 years ago

For now my incredibly awesome workaround is:

diff --git i/source/ceylon/ast/redhat/CaseTypes.ceylon w/source/ceylon/ast/redhat/CaseTypes.ceylon
index 4d9ff8b..e1cb29a 100644
--- i/source/ceylon/ast/redhat/CaseTypes.ceylon
+++ w/source/ceylon/ast/redhat/CaseTypes.ceylon
@@ -29,7 +29,7 @@ shared CaseTypes caseTypesToCeylon(JCaseTypes caseTypes, Anything(JNode, Node) u
      (This trick originally comes from ceylon.formatter.)
      */
     assert (nonempty cases
-                = concatenate(CeylonIterable(caseTypes.types), CeylonIterable(caseTypes.baseMemberExpressions))
+                = concatenate(CeylonIterable(caseTypes.types), CeylonIterable(caseTypes.baseMemberExpressions).narrow<JBaseMemberExpression>())
                     .sort(byIncreasing(compose(Token.tokenIndex, JNode.token))));
     PrimaryType|MemberName primaryTypeOrMemberNameToCeylon(JStaticType|JBaseMemberExpression that, Anything(JNode, Node) update = noop) {
         switch (that)
lucaswerkmeister commented 7 years ago

I’ve added your workaround for now (with you as Git author, since I literally just pasted your diff into git apply). Thanks for the suggestion! Hopefully I’ll get around to a proper fix at some point…

lucaswerkmeister commented 6 years ago

I’m postponing this issue, because it’s not clear to me what the proper fix should look like and I don’t want to block the 1.3.3 release any longer.