missing support for value constructors

[jvasileff]

Attempting to convert

            shared class A {
                shared new instance {}
            }

results in

Exception in thread "main" ceylon.language.AssertionError "Assertion failed
    violated is JMissingDeclaration|JTypeDeclaration|JTypedDeclaration|JConstructor|JTypeParameterDeclaration declaration"
    at ceylon.ast.redhat.declarationToCeylon_.declarationToCeylon(Declaration.ceylon:21)
    at ceylon.ast.redhat.declarationOrStatementToCeylon_.declarationOrStatementToCeylon(DeclarationOrStatement.ceylon:20)
    at ceylon.ast.redhat.classBodyToCeylon_$2.$call$(ClassBody.ceylon)
    at ceylon.ast.redhat.propagateUpdate_$1.$call$(propagateUpdate.ceylon:16)
    at ceylon.ast.redhat.propagateUpdate_$1.$call$(propagateUpdate.ceylon:16)
    at ceylon.language.Iterable$impl$3$1.next(Iterable.ceylon:357)
    at ceylon.language.Array.createArrayFromIterable(Array.java:157)
    at ceylon.language.Array.<init>(Array.java:106)
    at ceylon.language.Iterable$impl.sequence(Iterable.ceylon:251)
    at ceylon.language.impl.BaseIterable.sequence(bases.ceylon)
    at ceylon.language.Iterable$impl.collect(Iterable.ceylon:861)
    at ceylon.language.impl.BaseIterable.collect(bases.ceylon)
    at ceylon.ast.redhat.classBodyToCeylon_.classBodyToCeylon(ClassBody.ceylon:20)
    at ceylon.ast.redhat.classDefinitionToCeylon_.classDefinitionToCeylon(ClassDefinition.ceylon:27)
    at ceylon.ast.redhat.anyClassToCeylon_.anyClassToCeylon(AnyClass.ceylon:18)
    at ceylon.ast.redhat.classOrInterfaceToCeylon_.classOrInterfaceToCeylon(ClassOrInterface.ceylon:18)
    at ceylon.ast.redhat.typeDeclarationToCeylon_.typeDeclarationToCeylon(TypeDeclaration.ceylon:18)
    at ceylon.ast.redhat.declarationToCeylon_.declarationToCeylon(Declaration.ceylon:26)

the Redhat AST is:

-- Redhat AST 
+  [CompilationUnit] (2:0-4:0)
|  +  [ImportList]
|  + class [ClassDefinition] (2:0-4:0) : class A
|  |  +  [AnnotationList] (2:0-2:5)
|  |  |  +  [Annotation] (2:0-2:5) : SharedAnnotation
|  |  |  |  +  [BaseMemberExpression] (2:0-2:5) : SharedAnnotation() : shared : function shared() => SharedAnnotation
|  |  |  |  |  + shared [Identifier] (2:0-2:5)
|  |  |  |  |  +  [InferredTypeArguments]
|  |  |  |  +  [PositionalArgumentList]
|  |  + A [Identifier] (2:13-2:13)
|  |  + {} [ClassBody] (2:15-4:0)
|  |  |  + new [Enumerated] (3:4-3:25) : value A.instance => A.instance
|  |  |  |  +  [AnnotationList] (3:4-3:9)
|  |  |  |  |  +  [Annotation] (3:4-3:9) : SharedAnnotation
|  |  |  |  |  |  +  [BaseMemberExpression] (3:4-3:9) : SharedAnnotation() : shared : function shared() => SharedAnnotation
|  |  |  |  |  |  |  + shared [Identifier] (3:4-3:9)
|  |  |  |  |  |  |  +  [InferredTypeArguments]
|  |  |  |  |  |  +  [PositionalArgumentList]
|  |  |  |  + instance [Identifier] (3:15-3:22)
|  |  |  |  + {} [Block] (3:24-3:25)

[lucaswerkmeister]

Actually, it looks like we just don’t support value constructors at all – I seem to have completely forgotten that. Whoops :)

Spec:

CallableConstructorHeader: "new" MemberName? Parameters ExtendedType?
ValueConstructorHeader: "new" MemberName ExtendedType?

I thought about just making ConstructorDefinition’s parameters optional, but then we can’t encode the requirement that a value constructor must have a name, so it’s probably better to have separate classes: rename ConstructorDefinition to CallableConstructorDefinition, add abstract superclass ConstructorDefinition, add subclass ValueConstructorDefinition. Objections?

[jvasileff]

That sounds reasonable to me. The only counter arguments I can come up with are:

1. You also can't have multiple callable constructors with no name or matching names (not a syntactic issue, but still a practical concern for users)
2. LazySpecification is used for both functions and values, so just having one class would be parallel.
3. If you'll consider issuing patch releases for 1.2.0 :smile: the optional parameters approach would result in a less jarring compatibility change, if that even matters.

But overall, I think I agree that having name for values be non-null is the better approach.

[lucaswerkmeister]

If you'll consider issuing patch releases for 1.2.0

That’s the plan, yes, to put this in a 1.2.1 or something.

[lucaswerkmeister]

(note to self: don’t forget to do this in a branch based on 1.2.0, rather than master.)

[lucaswerkmeister]

Done in branch addValueConstructorDefinition. @jvasileff can you try it out before I push the merge to master? (Also cc @Vorlent, this should fix your problem from Dec 6th.)

[lucaswerkmeister]

Well, I tried to run the Dart backend with ceylon.ast on addValueConstructorDefinition… and I can confirm that the reported bug no longer occurs. Instead, it naturally blows up with an error that transformValueConstructorDefinition isn’t implemented in captureVisitor.

I’ll merge it into master, since it looks mostly good to me.

[jvasileff]

Thanks @lucaswerkmeister, that was still on my todo list.

naturally blows up with an error

that's not natural, it's horrible!

[jvasileff]

BTW, with the latest couple fixes, we've got to be close to 100% coverage for real world testing. It's amazing how well ceylon.ast has worked out!

[lucaswerkmeister]

that's not natural, it's horrible!

well I added a method to the interface and then ran your compiler again without recompiling it – what else could possibly have happened? I’m not sure how I didn’t realize this would happen before I tried it out.

[jvasileff]

Yeah, it's not all that unexpected. But my aim has been to provide nice compiler error messages with column and line number info for unsupported features.

edit: never mind :smile: