Support for Type[length]

[lucaswerkmeister]

We need to support ceylon/ceylon-spec#1041.

I’ll probably wait until it’s properly specified though.

[lucaswerkmeister]

This has now been declared “an unofficial hidden feature of 1.1”, so ceylon.ast support may slip to 1.2.

[lucaswerkmeister]

Moving to 1.2; if I added it now, I would just have to update it later for consistency with the spec anyways.

[lucaswerkmeister]

Alright, these have been added to the spec as

TupleType: "[" TypeList "]" | PrimaryType "[" DecimalLiteral "]"

How do we model them?

My best idea currently would be to turn TupleType into an abstract class with the two subclasses ListTupleType and LengthTupleType, but I don’t really like the sound of that too much.

[jvasileff]

A three class solution sounds right to me.

[lucaswerkmeister]

I implemented exactly what I proposed in the above comment, since I haven’t been able to think of anything better (neither layout nor name) in the meantime. Done.

[lucaswerkmeister]

Fuck me, I already did this half a year ago in c4329c1 without mentioning this issue. So now I have two implementations of this and need to decide which to keep.

[lucaswerkmeister]

So we currently have two implementations of this:

1. SequentialType has an optional length child of type IntegerLiteral
2. TupleType is an abstract supertype of ListTupleType (previously TupleType) and LengthTupleType (PrimaryType plus IntegerLiteral).

Which one should we keep? 1 is closer to the RedHat AST, while 2 is slightly closer to the spec.

I think I lean slightly towards 2. Any other opinions?

[lucaswerkmeister]

What’s really mysterious, by the way, is this: both implementations have tests, and all tests currently pass. I don’t understand how this is possible, since both versions should erase to the same RedHat AST – and consequently, it shouldn’t be possible to distinguish between them, and both versions should be converted back to the same version (I would expect 2 due to this). I’ll definitely have to look into this too.

[jvasileff]

I'd lean towards 2, I think, since it is indeed a ceylon.language::Tuple. But of course, that's not absolute. Sequence, Sequential, and Tuple are often used interchangeably. Notably, in ceylon.ast, ceylon.language::Empty is represented by a TupleType, even though it is not a ceylon.language::Tuple.

Oh, wait, no. I change my vote. It seems SequentialType is of the form Type[whatever] and TupleType is of the form [whatever]. So String[2] would be of the SequentialType form.

[lucaswerkmeister]

I guess that from the viewpoint that ceylon.ast is about syntax, not semantics, 1 makes more sense…

[lucaswerkmeister]

Found out why the tests work: primaryType and type test objects are completely missing. No one is testing typeToCeylon. Probably related to #100. screams in horror

I’ll add those, check that the tests fail as they should, then remove solution 2.

[lucaswerkmeister]

Done again. Once I fixed the tests (9487f13), they indeed failed as expected, so now I know they work :) and once I removed 2 in ae4c94d, they worked again.

[lucaswerkmeister]

Reopening again because there’s one last open question. Per spec, only decimal integer literals are allowed, not binary or hex ones:

TupleType: "[" TypeList "]" | PrimaryType "[" DecimalLiteral "]"

I enforced this in implementation 2 (here), but not in implementation 1. Should I add it to implementation 1? It depends on whether this is considered a syntax error or a different error. It looks like a syntax error in the spec, but the compiler grammar accepts any integer literal, and instead of a syntax error you get an “error: must be a positive decimal integer”, probably from LiteralVisitor.

I could live with both; probably leaning slightly towards not adding the check (but documenting that).

[lucaswerkmeister]

Not checking.