Fix #5495: Reject invalid modifiers on enums

[allanrenucci]

Fixes #5034

[allanrenucci]

Note the toTypeFlags here. It is to prevent a illegal flagset combination assertion failure that occurs when combining TermFlags with TypeFlags. For example when parsing:

lazy enum Foo {}

[odersky]

The usual place to check flags is in Checking.checkWellformed. I believe we should do any checking for enum modifiers there. No changes to Parser should be necessary.

[allanrenucci]

I checked the flags in the parser so that they are sanitised before desugaring happens. Otherwise we report spurious errors:

scala> implicit enum Foo {}                                                                                                                                                                             
1 |implicit enum Foo {}
  |^^^^^^^^^^^^^^^^^^^^
  |Implicit classes must accept exactly one primary constructor parameter

scala> implicit enum Foo(x: Int) {}
1 |implicit enum Foo(x: Int) {}
  |              ^
  |              implicit modifier cannot be used for types or traits

Also, if flags are not sanitised before desugaring, this may lead to illegal flag combination:

scala> lazy enum Foo{}
Exception in thread "main" java.lang.AssertionError: assertion failed: illegal flagset combination: lazy abstract <enum> and sealed
	at dotty.DottyPredef$.assertFail(DottyPredef.scala:38)
	at dotty.tools.dotc.core.Flags$FlagSet$.$bar$extension(Flags.scala:26)
	at dotty.tools.dotc.ast.DesugarEnums$.addEnumFlags(DesugarEnums.scala:72)

[odersky]

There are lots of situations where some flag combinations are illegal. That's why we check them in two centralized places: checkFlags and checkWellformed. The former just checks that we don't mix type flags and term flags and the latter does all the other checks. I believe it's unsystematic to now also check some flag combinations in parser.

So, whatever we do, let's check all flags in the same way! Maybe we decide it's best to check all flags in parser, or in desugaring, or in Namer, as it is done now. But let's not create exceptions for specific cases.

In fact I believe checking flags in desugaring instead of Namer might be a reasonable compromise.

[allanrenucci]

I am unassigning myself. I will not have time to address the review comments. Feel free to close or take over the PR

[odersky]

@allanrenucci Understood. Thanks for your work on this, which will be useful for me to take over.

[Blaisorblade]

@odersky That comment applies to #4973 as well apparently, and overrides @smarter's suggestion there, I guess?

So, whatever we do, let's check all flags in the same way! Maybe we decide it's best to check all flags in parser, or in desugaring, or in Namer, as it is done now. But let's not create exceptions for specific cases.

In fact I believe checking flags in desugaring instead of Namer might be a reasonable compromise.

Does the choice affect the IDE behavior on erroneous programs?

[Blaisorblade]

Does the choice affect the IDE behavior on erroneous programs?

Of course that affects whether you get other errors, but these ones should be easy to fix. I asked Olaf, he doesn't have a strong opinion.

[odersky]

Superseded by #7850