Add "enum" construct #1958

odersky · 2017-02-08T11:29:18Z

This is a prototype implementation to add an "enum" construct to Scala. Scala enums give a more concise notation for

enums as in Java
ADTs
GADTs

A specification of what's implemented is provided by #1970.

Current status:

First implementation
Some test cases
A specification
A decision whether we want to go ahead with this

If we want to go ahead with this, the following todos should be broken out as separate issues.

An implementation of generic programming in the style of SYB. We need to clarify first exactly
what we want from the compiler
A way to interop with Java enumerations
Update the exhaustiveness checking in the pattern matcher
Optimize pattern matcher to use enumTags

viktorklang · 2017-02-08T12:23:30Z

tests/run/enum-Color.scala

+  case Red
+  case Green
+  case Blue
+}


Will

enum Color { case Red, Green, Blue }

expand to the equivalent of the above?

Not yet. I was thinking it would be a nice abbreviation, but not decided yet.

enum Color { case Red, Green, Blue } is the word "case" needed in the abbreviation?

We definitely should keep case

odersky · 2017-02-08T23:12:14Z

@felixmulder The build failure comes from my changes to how doc comments are treated in Parser. The changes are in b20c1be. I noted a lot of docstring parameters passed, but they were all in perfect sync with the start position that we also pass, and they were all computed as in.getDocString(start). I foolishly assumed that getDocString was referentially transparent and could be called at the point where we need the docstring instead. But that seems to be a wrong assumption. My attempted fix in 4e0dc0e did not help.

So, we could back out, which is slightly messy but doable. But maybe it's best to go forward instead. Can you make getDocString referentially transparent, so that the timing when we call it does not matter? I would simply manage a map from a token start offset to the docstring that immediately precedes it. Then, since we have exact start positions of all trees we automatically have their docstrings as well, and a lot of redundancy and passing things around becomes unnecessary. I also don't see a need in this case to maintain a stack of docstrings that follows the block structure. WDYT?

felixmulder · 2017-02-09T09:24:26Z

I think that originally I wanted to protect from all cases of misuse. E.g:

trait T {
  /** Cheeky docstring that shouldn't appear in `C` */
}
class C

ce40517 is a bit more permissive and allows this misuse...

Another alternative to this fix could be to make docstring its own token and attach it as part of the AST, but I guess that would be added complexity to the parser.

Update: failing test is compileStdLib

odersky · 2017-02-09T11:41:41Z

I think that originally I wanted to protect from all cases of misuse. E.g:

trait T {
   /** Cheeky docstring that shouldn't appear in `C` */
}
class C

But if you associate doc comments to start positions of tokens following them, you can do that. The cheeky doc comment would be associated with }' not with class`. So it would not qualify.

Re: stdlib I forgot to push a new one. (the old one uses enum in one place, but that works no longer since it is now a keyword).

viktorklang · 2017-02-13T08:31:16Z

+💯

…

-- Cheers, √

On Feb 13, 2017 9:24 AM, "odersky" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In tests/run/enum-Color.scala <#1958>: > @@ -0,0 +1,13 @@ +enum Color { + case Red + case Green + case Blue +} We definitely should keep case — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1958>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAqd8X7-GsxMsflab2gSzqg8ToqpRc0ks5rcBMsgaJpZM4L6s5a> .

odersky · 2017-02-14T14:23:48Z

Regarding shorthand for simple enums: Instead of

enum Color { case Red, Green, Blue }

should it rather be:

enum Color { case Red | Green | Blue }

? That would align it with pattern syntax.

odersky · 2017-02-27T18:03:15Z

We now have the syntax

enum Color { case Red, Green, Blue }

supported.

I propose we defer all other todos to separate issues and skip straight to the decision. Do we want to go ahead with this concept? If everyone is still positive, let's review the code and get this in.

liufengyun

Otherwise, LGTM

liufengyun · 2017-02-28T13:42:20Z

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

+        ModuleDef(modName, Template(emptyConstructor, Nil, EmptyValDef, body))
+          .withMods(mods)
+      }
+      Thicket(clsDef :: modDef :: Nil)


I'm not sure if it's a good idea to do this inside parser.

When we introduce macros, we do have multiple level desugaring (expansion) in Namer. And as we already have all the facility to support multiple level desugaring, I think we probably should use that. I've done some changes in Namer to support multiple-level desugaring can be seen here. It seems a flatten method is all that's needed.

I tried multi-level expansion first, including flattening, but it did not work. Essentially the compiler got confused with symbol attachments and I did not find a good way around it.

liufengyun · 2017-02-28T14:57:49Z

compiler/src/dotty/tools/dotc/ast/Desugar.scala

+    val result = flatTree(cdef1 :: companions ::: implicitWrappers)
+    //if (isEnum) println(i"enum $cdef\n --->\n$result")
+    //if (isEnumCase) println(i"enum case $cdef\n --->\n$result")
+    result


A little cleanup is required here.

liufengyun · 2017-02-28T15:09:24Z

compiler/src/dotty/tools/dotc/ast/Desugar.scala

            // todo: also use anyRef if constructor has a dependent method type (or rule that out)!
-          else (constrVparamss :\ classTypeRef) ((vparams, restpe) => Function(vparams map (_.tpt), restpe))
+            (constrVparamss :\ (if (isEnumCase) applyResultTpt else classTypeRef)) (
+              (vparams, restpe) => Function(vparams map (_.tpt), restpe))


I may miss some point here, it seems to me the fold is useless here, because constrVparamss can only be of size 0?

size 0 or 1

size 0 or 1, actually.

Ah, I see, thanks, obviously I've some problems with basic math :)

liufengyun · 2017-02-28T15:23:40Z

compiler/src/dotty/tools/dotc/ast/Desugar.scala

+    val originalTparams =
+      if (isEnumCase && parents.isEmpty) {
+        if (constr1.tparams.nonEmpty)
+          ctx.error(em"case with type parameters needs extends clause", constr1.tparams.head.pos)


What if the parent takes no type parameters, while the child has type parameters, like the following case:

enum Base { case Bar[T](x: T) }

Good point. An example for this are HLists:

enum HList { case HCons[Hd, Tl](hd: Hd, tl: Tl) case HNil }

I think we want to allow this.

odersky · 2017-03-02T10:41:54Z

Rebased to master

Insert an empty line before "where" in an explanation.

Modify syntax.md and Tokens/Parser/untpd to support enums.

It's completely redundant, docstring is just the comment found at the `start` offset, which is passed anyway.

`enum' only allowed as a prefix of classes, dropped from traits and objects.

The previous scheme did not work because desugaring cannot deal with repeated expansions. We now sidestep the issue by doing the expansion in the parser. Luckily, positions work out perfectly, so that one can reconstruct the source precisely from the parsed untyped trees.

In case of difference, dump transcript to file for easier comparisons.

Flags like Trait are in fact not always defined when a symbol is created. For symbols loaded from class files, this flag, and some other is defined only once the classfile has been loaded. But this happens in general before the symbol is completed. We model this distinction by separating from the `FromStartFlags` set a new set `AfterLoadFlags` and distinguishing between the two sets in `SymDenotations#is`. Test case is enum-Option.scala. This erroneously complained before that `Enum` was not a trait.

`copy` should always return the type of it's rhs. The discussion of scala#1970 concluded that no special treatment for enums is needed.

In an enum case like case C() extends P1 with ... with Pn ... apply now returns `P1 & ... & Pn`, where before it was just P1. Also, add to Option test.

Based on the discussion in scala#1970, enumeration objects now have three public members: - valueOf: Map[Int, E] - withName: Map[String, E] - values: Iterable[E] Also, the variance of case type parameters is now the same as in the corresponding type parameter of the enum class.

Support cases with type parameters that implicitly extend a non-parameterized base without needing their own extends clause. The proposal has been updated to make clear that this is supported. Also address other reviewers comments.

Previous expansion caused 3 spurious follow-on errors which are now avoided.

This commit can hopefully be reverted once scala#2121 is in.

Infer type arguments for enum paraments from corresponding type parameter bounds. This only works if the type parameter in question is variant and its bound is ground.

- rename utility methods - generate utility methods also for object cases

odersky · 2017-04-05T15:15:19Z

As far is I'm concerned we are done for this part. We already had a review by @liufengyun but if people want to review either everything or just the later commits please assign to yourself as reviewer.

liufengyun · 2017-04-05T15:33:14Z

One thing missing is exhaustivity check, which can be done in another PR.

According to our current algorithm, as long as the parent class symbol carries a list of children symbols (case vals and case classes), the algorithm will be able to handle.

My concern here is that in parser, it's clear what the children are, and it's easy to get them. But after desugaring, it seems to require a lot of assumptions about desugaring in order to recover that information.

viktorklang reviewed Feb 8, 2017

View reviewed changes

odersky mentioned this pull request Feb 13, 2017

Add enum construct #1970

Closed

odersky force-pushed the add-enum branch from d59313d to 6d6e88c Compare February 27, 2017 16:44

odersky requested a review from liufengyun February 28, 2017 10:58

liufengyun reviewed Feb 28, 2017

View reviewed changes

odersky force-pushed the add-enum branch from b6df668 to 87e9eba Compare March 2, 2017 10:41

odersky changed the title ~~WIP Add "enum" construct~~ Add "enum" construct Mar 3, 2017

LPTK mentioned this pull request Mar 6, 2017

[enum construct] Name-based rules for implicit extension #2055

Closed

felixmulder added the stat:in progress label Apr 4, 2017

odersky and others added 14 commits April 4, 2017 13:20

Fix mal-formatting.

0a48911

Insert an empty line before "where" in an explanation.

Add enum syntax

669c5a8

Modify syntax.md and Tokens/Parser/untpd to support enums.

Don't pass docstring as a parameter.

ca039ba

It's completely redundant, docstring is just the comment found at the `start` offset, which is passed anyway.

Simplify syntax

fe14afb

`enum' only allowed as a prefix of classes, dropped from traits and objects.

Allow value expansion of modules in mergeCompanionDefs

5c53f53

Adapt generic tests to model modified enum values scheme

69fd9dc

Implement enum desugaring

cea243a

Add tests

c279057

Comment out unused method in Context

b4ece2a

Improvement to REPL test

db387a2

In case of difference, dump transcript to file for easier comparisons.

Fix "closest" computation for docstrings

3ea9831

Make getDocComment referentially transparent

adf9009

Another test

2be70e6

odersky and others added 14 commits April 4, 2017 13:29

New test case

d3b2c37

Fix typo

8f3c9a8

Fix cheeky comment in nested scope

cb3e536

Fix typo in syntax

1196533

Check that cases with type parameters also have an extends clause

a30e7ec

Don't change the return type of the copy method

c58555e

`copy` should always return the type of it's rhs. The discussion of scala#1970 concluded that no special treatment for enums is needed.

Change return type of apply.

4bdad3c

In an enum case like case C() extends P1 with ... with Pn ... apply now returns `P1 & ... & Pn`, where before it was just P1. Also, add to Option test.

Support comma-separated enum constants

91a26b3

Fix neg test error count

62b4eb8

Previous expansion caused 3 spurious follow-on errors which are now avoided.

Avoid assertion failure on neg test

4c576bf

This commit can hopefully be reverted once scala#2121 is in.

Fix rebase breakage

1c79612

odersky force-pushed the add-enum branch from 87e9eba to a46f4f8 Compare April 4, 2017 16:30

odersky added 4 commits April 4, 2017 18:31

Infer enum type args from type parameter bounds

a46f4f8

Infer type arguments for enum paraments from corresponding type parameter bounds. This only works if the type parameter in question is variant and its bound is ground.

New and updated tests

9b37a7c

Add check file

9938344

Implementation of proposal changes

f702773

- rename utility methods - generate utility methods also for object cases

odersky added stat:needs review and removed stat:in progress labels Apr 5, 2017

odersky added 2 commits April 5, 2017 17:21

Update test and add check file

9201896

Another test fixed

18d0ae5

odersky mentioned this pull request Apr 5, 2017

Add enum exhaustivity checking #2197

Merged

Emit enum utility methods only if enum class is not generic

30d8d87

odersky merged commit 62c2a1e into scala:master Apr 6, 2017

allanrenucci deleted the add-enum branch December 14, 2017 16:57

OlivierBlanvillain removed the stat:needs review label Mar 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "enum" construct #1958

Add "enum" construct #1958

odersky commented Feb 8, 2017 •

edited

Loading

viktorklang Feb 8, 2017

odersky Feb 8, 2017

vicaba Feb 9, 2017

odersky Feb 13, 2017

odersky commented Feb 8, 2017 •

edited

Loading

felixmulder commented Feb 9, 2017 •

edited

Loading

odersky commented Feb 9, 2017 •

edited

Loading

viktorklang commented Feb 13, 2017 via email

odersky commented Feb 14, 2017

odersky commented Feb 27, 2017

liufengyun left a comment

liufengyun Feb 28, 2017

odersky Feb 28, 2017

liufengyun Feb 28, 2017

liufengyun Feb 28, 2017

odersky Feb 28, 2017

odersky Feb 28, 2017

liufengyun Feb 28, 2017

liufengyun Feb 28, 2017

odersky Feb 28, 2017

odersky commented Mar 2, 2017

odersky commented Apr 5, 2017

liufengyun commented Apr 5, 2017

Add "enum" construct #1958

Add "enum" construct #1958

Conversation

odersky commented Feb 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odersky commented Feb 8, 2017 • edited Loading

felixmulder commented Feb 9, 2017 • edited Loading

odersky commented Feb 9, 2017 • edited Loading

viktorklang commented Feb 13, 2017 via email

odersky commented Feb 14, 2017

odersky commented Feb 27, 2017

liufengyun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odersky commented Mar 2, 2017

odersky commented Apr 5, 2017

liufengyun commented Apr 5, 2017

odersky commented Feb 8, 2017 •

edited

Loading

odersky commented Feb 8, 2017 •

edited

Loading

felixmulder commented Feb 9, 2017 •

edited

Loading

odersky commented Feb 9, 2017 •

edited

Loading