ast destructuring syntax more restrictive than typechecker's

[jvasileff]

The type checker allows:

    String[2] tup = ["one", "two"];
    switch(tup)
    case ([a,b]) { print(b); }

where we really want case ([String a, String b]) {...}, but ceylon.ast throws:

Exception in thread "main" ceylon.language.AssertionError "Assertion failed
    violated is JStaticType jType = isCase.type"
    at ceylon.ast.redhat.isCaseToCeylon_.isCaseToCeylon(IsCase.ceylon:15)
    at ceylon.ast.redhat.caseItemToCeylon_.caseItemToCeylon(CaseItem.ceylon:20)
    at ceylon.ast.redhat.caseClauseToCeylon_.caseClauseToCeylon(CaseClause.ceylon:14)
    at ceylon.ast.redhat.switchCasesToCeylon_$2.$call$(SwitchCases.ceylon)
    at ceylon.ast.redhat.switchCasesToCeylon_$2.$call$(SwitchCases.ceylon:18)

[jvasileff]

See also https://github.com/ceylon/ceylon/issues/6330, which makes use of this syntax.

[lucaswerkmeister]

Wait, why is this an isCase?

[gavinking]

Because it gets desugared.

[lucaswerkmeister]

But there’s no is token. Is this in the spec?

[gavinking]

What? Why would desugaring be in the spec?

[lucaswerkmeister]

What, since when is it not the spec’s job to describe the full and complete language and everything that users can do in it? What do I care that it’s a desugaring? exists and nonempty conditions could also be implemented as simple desugaring (is Object and is Sequence<Anything>), how the compiler actually does it is an implementation detail that shouldn’t matter to the spec.

[gavinking]

I've no idea who you're arguing with.

[lucaswerkmeister]

You, since you seem to be saying that the case ([a,b]) syntax doesn’t need to be documented in the spec. Or did I misunderstand that?

[gavinking]

Where on earth did I say that?!

[lucaswerkmeister]

Why would desugaring be in the spec?

[gavinking]

@lucaswerkmeister do you know what "desugaring" means?

[lucaswerkmeister]

I might have misinterpreted “why would desugaring be in the spec” as “why would the spec tell you how to desugar this”, i. e. “why would sugaring be in the spec”. Not sure :D

In any event… do you plan to add this syntax to the spec? Is it already there and I’m not seeing it?

[gavinking]

Look, none of the new destructuring functionality in 1.2.3 (in case, or in anon function parameter lists) is covered at all in the spec yet, which is one reason why the relevant issues have not yet been closed.

But the desugaring of a PatternCase to an IsCase followed by a Destructure is not likely how I will define it when I get around to writing it up. Instead, the spec will define a new (third) kind of case condition.

[gavinking]

FTR, I don't even love this implementation strategy. I did it this way (i.e. using desugaring) only because I didn't want to impact the backends.

[lucaswerkmeister]

But the desugaring of a PatternCase to an IsCase followed by a Destructure is not likely how I will define it when I get around to writing it up. Instead, the spec will define a new (third) kind of case condition.

This is also how I would prefer to model it in ceylon.ast. But if I add that now, I’ll probably have to change it once you write the spec for it (to keep the two in sync), which is why I usually wait for the spec before adopting changes in ceylon.ast. (#114 is pretty much the same situation afaict, right up to the point where it seems you don’t love the current implementation strategy.) The problem is that this is bad news for @jvasileff :-/

[gavinking]

Many times I have argued with @FroMage that we should not implement things using desugaring because it's bad for tools. Information is lost.

[jvasileff]

Looking into this further, case ([String a, String b]) { } does work, and the rh-ast is:

|  |  |  |  +  [SwitchCaseList] (4:4-4:44)
|  |  |  |  |  + case [CaseClause] (4:4-4:44)
|  |  |  |  |  |  +  [IsCase]
|  |  |  |  |  |  |  +  [TupleType] : String[2]
|  |  |  |  |  |  |  |  +  [BaseType] (4:11-4:16) : String : class String({Character*} characters)
|  |  |  |  |  |  |  |  |  + String [Identifier] (4:11-4:16)
|  |  |  |  |  |  |  |  +  [BaseType] (4:21-4:26) : String : class String({Character*} characters)
|  |  |  |  |  |  |  |  |  + String [Identifier] (4:21-4:26)
|  |  |  |  |  |  |  +  [Variable] : value tup => String[2]
|  |  |  |  |  |  |  |  + tup [Identifier]
|  |  |  |  |  |  |  |  +  [SyntheticVariable] : String[2]
|  |  |  |  |  |  |  |  +  [SpecifierExpression]
|  |  |  |  |  |  |  |  |  +  [Expression] : String[2]
|  |  |  |  |  |  |  |  |  |  +  [BaseMemberExpression] : String[2] : tup : value run.tup => String[2]
|  |  |  |  |  |  |  |  |  |  |  + tup [Identifier]
|  |  |  |  |  |  |  |  |  |  |  +  [InferredTypeArguments]
|  |  |  |  |  |  + {} [Block] (4:32-4:44)
|  |  |  |  |  |  |  +  [Destructure] (4:10-4:29)

but with case ([a, b]) { }, the rh-ast isn't as complete:

|  |  |  |  +  [SwitchCaseList] (4:4-4:30)
|  |  |  |  |  + case [CaseClause] (4:4-4:30)
|  |  |  |  |  |  +  [IsCase]
|  |  |  |  |  |  |  +  [ValueModifier] : unknown
|  |  |  |  |  |  |  +  [Variable] : value tup => String[2]
|  |  |  |  |  |  |  |  + tup [Identifier]
|  |  |  |  |  |  |  |  +  [SyntheticVariable] : String[2]
|  |  |  |  |  |  |  |  +  [SpecifierExpression]
|  |  |  |  |  |  |  |  |  +  [Expression] : String[2]
|  |  |  |  |  |  |  |  |  |  +  [BaseMemberExpression] : String[2] : tup : value run.tup => String[2]
|  |  |  |  |  |  |  |  |  |  |  + tup [Identifier]
|  |  |  |  |  |  |  |  |  |  |  +  [InferredTypeArguments]
|  |  |  |  |  |  + {} [Block] (4:18-4:30)
|  |  |  |  |  |  |  +  [Destructure] (4:10-4:15)

Maybe, the fixes for this and https://github.com/ceylon/ceylon/issues/6330 are the same.

[jvasileff]

Regarding the overall approach, it might be nice to typecheck without de-sugaring, and then have an extra de-sugaring visitor (for each feature) that can optionally be used by the backends.

[jvasileff]

Aside from having the typechecker produce a valid Redhat AST, I don't think any of the discussed options are all that great.

And I really wouldn't look forward to trying to have the Dart backend reverse engineer ceylon.asts reverse engineering of the typechecker's desugaring, with none of that work being held to any standard of quality or guaranteeing compatibility across releases.

For my purposes, I'd be perfectly fine having the Dart backend be responsible for replacing is value with something that I can recognize and that will be accepted, and letting ceylon.ast.redhat ignore the problem.

That leaves other users of ceylon.ast.redhat possibly running into this, but it is a very unusual construct anyway, and all of this will become irrelevant if/when matching is enhanced.

[jvasileff]

Besides, it might be nice for me to have code that is known to blow up the backend for use when testing error handling in tools 😄