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

allow mix-and-match U/LIdentifiers for ImportElements #122

Closed
jvasileff opened this issue Sep 26, 2016 · 7 comments
Closed

allow mix-and-match U/LIdentifiers for ImportElements #122

jvasileff opened this issue Sep 26, 2016 · 7 comments

Comments

@jvasileff
Copy link
Member

jvasileff commented Sep 26, 2016

I have it on good authority that the following is valid Ceylon:

import simple {
    UClass=lclass
}

class \Ilclass() {}

but ceylon.ast.redhat rejects this:

Exception in thread "main" ceylon.language.AssertionError "Assertion failed: Must b                      
e LIDENTIFIER token                                                                                      
        violated token.type == lidentifier"                                                              
        at ceylon.ast.redhat.lIdentifierToCeylon_.lIdentifierToCeylon(Identifier.ce                      
ylon:60)                                                                                                 
        at ceylon.ast.redhat.importFunctionValueAliasToCeylon_.importFunctionValueA                      
liasToCeylon(ImportFunctionValueAlias.ceylon:19)                                                         
        at ceylon.ast.redhat.importFunctionValueElementToCeylon_.importFunctionValu                      
eElementToCeylon(ImportFunctionValueElement.ceylon:21)                                                   
        at ceylon.ast.redhat.importElementToCeylon_.importElementToCeylon(ImportEle                      
ment.ceylon:23)                                                                                          
        at ceylon.ast.redhat.importElementsToCeylon_$1.$call$(ImportElements.ceylon                      
)                                                                                                        
        at ceylon.ast.redhat.importElementsToCeylon_$1.$call$(ImportElements.ceylon                      
:18)                                                                                                     
@lucaswerkmeister
Copy link
Member

The comment you cite also says:

Now, sure, the spec doesn't bless this, because it considers Java interop to be out of scope.

And as I’ve explained before, ceylon.ast’s current goal is to reflect the specification, not necessarily all the interop extensions the compiler also supports. Feel free to discuss that in #123, but this issue (#122) is not currently a bug.

@lucaswerkmeister
Copy link
Member

Okay, so when we go through with #123, how should this be fixed?

  • Completely merge the ImportType- and ImportFunctionValue- subclasses of the no-longer-abstract ImportElement and ImportAlias. There is only one kind of import element, and both sides can be uppercase or lowercase identifiers.
  • Merge the ImportAlias subclasses, but keep the ImportElement classes separate. There are still two kinds of import element, but the right-hand side can be uppercase or lowercase identifier.

I’m leaning towards the second one.

@jvasileff
Copy link
Member Author

jvasileff commented Oct 10, 2016

IIUC, option 2 suggests a semantic difference between ImportTypeElement and ImportFunctionValueElement where none exists, since a) as we've established the case doesn't matter when the import is aliased, and b) the typechecker doesn't even care about the case when not aliased:

import simple {
    lowerClass, // no error
    UpperFunction // no error
} 

class \IlowerClass() {}
void \iUpperFunction() {}

I believe the only thing that is actually checked by the typechecker is ImportAlias.identifier.

So it seems the most consistent approach would be to keep ImportTypeAlias | ImportFunctionValueAlias, but merge ImportTypeElement | ImportFunctionValueElement.

All of this seems quite arbitrary though, based on what the typechecker currently does, so I guess I'd lean towards option 1.

@lucaswerkmeister
Copy link
Member

Hm, I assumed that the typechecker established this semantic difference at least on the left-hand side, that is:

import java.something {
    ProperClassName=classThatIsLowercaseForSomeStupidReason {
        properValueName=UppercaseValueNameBecauseReasons,
        UppercaseValueName=noStopWhatAreYouDoing // not allowed!
    },
    badClassName=WhyDoYouDoThis // not allowed!
}

I don’t think keeping the aliases separate is worth much, so option 1 it is, I suppose.

@lucaswerkmeister
Copy link
Member

LOL, look what I wrote two years ago:

Perhaps we should just drop the uppercase / lowercase distinction from both specification and tree?

lucaswerkmeister added a commit that referenced this issue Nov 25, 2016
ImportElement and ImportAlias are now each a single class, the
case-specific subclasses have been merged and are gone.
@lucaswerkmeister
Copy link
Member

Done on branch mergeImports.

@lucaswerkmeister
Copy link
Member

And now merged into master.

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

2 participants