Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for of package.foo #134

Open
lucaswerkmeister opened this issue Apr 4, 2017 · 9 comments
Open

Add support for of package.foo #134

lucaswerkmeister opened this issue Apr 4, 2017 · 9 comments

Comments

@lucaswerkmeister
Copy link
Member

See eclipse-archived/ceylon#4368 and eclipse-archived/ceylon@7a102d2.

@lucaswerkmeister
Copy link
Member Author

Ouch, this even broke the build…

@lucaswerkmeister
Copy link
Member Author

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
Copy link
Member

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

@jvasileff
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

+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
Copy link
Member

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)

jvasileff added a commit to jvasileff/ceylon-dart that referenced this issue Jul 29, 2017
lucaswerkmeister pushed a commit that referenced this issue Sep 28, 2017
Until we properly implement #134, this hack (copied from @jvasileff’s
GitHub comment [1]) at least lets ceylon.ast.redhat compile.

[1]: #134 (comment)
@lucaswerkmeister
Copy link
Member Author

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
Copy link
Member Author

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.

@lucaswerkmeister lucaswerkmeister modified the milestone: Next release Nov 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants