scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.85k stars 1.06k forks source link

extending java.lang.Enum should be desugared before TASTy #7478

Open bishabosha opened 4 years ago

bishabosha commented 4 years ago

minimized code

Compile with 0.27.0-RC1

enum A extends java.lang.Enum[A]
  case A1

In TASTy, the first parent of A is

TypeApply(Select(New(java.lang.Enum[A]),<init>[Signed (1)java.lang.Enum]), List(A))

with no value arguments passed.

expectation

The parent in TASTy should be

Apply(TypeApply(Select(New(java.lang.Enum[A]),<init>[Signed (1,java.lang.String,scala.Int)java.lang.Enum]), List(A)), List($name, _$ordinal))

with any other relevant desugarings applied, such as adding $name and _$ordinal to the constructor of A

Alternative

special-casing the synthetic no-arg java.lang.Enum constructor becomes part of the specification for TASTy

bishabosha commented 4 years ago

This one seems more tricky, if we move the phase CompleteJavaEnums before pickling, then the check fails for the @static annotated forwarders to enum values after moving them to the enum class, we can either keep the forwarder moving in a later phase, or special case the error for @static, or special case pickling for this signature, or do nothing.

@smarter @sjrd

smarter commented 4 years ago

if we move the phase CompleteJavaEnums before pickling

We can't have a phase that changes signature before pickling anyway: https://github.com/lampepfl/dotty/commit/1818b99f7cec44e70ea75eb41e63e4fd0b7d7647

bishabosha commented 4 years ago

we could chalk it up to another special case primitive of scala implementations? ala Nothing, &, |, AnyKind etc,

bishabosha commented 4 years ago

~I imagine this leaks into tasty reflect also so that could be a lot of special casing~ Edit: seems reflection doesn't reify apply nodes for parents

bishabosha commented 4 years ago

I am going to special case signatures in pickling and then benchmark it

bishabosha commented 4 years ago

On second thought, it just makes no sense to patch extends java.lang.Enum[MyEnum] with the correct signature without also expanding the tree to extends java.lang.Enum[MyEnum]($name, $ordinal), where $name and $ordinal do not exist yet. So again making this change seems pretty dubious without the complete desugaring of constructors appearing before tasty, which is more tricky because there is no precedent for patching a specific parent in desugaring