Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid final object #11094

Open
Blaisorblade opened this issue Aug 21, 2018 · 6 comments
Open

Forbid final object #11094

Blaisorblade opened this issue Aug 21, 2018 · 6 comments

Comments

@Blaisorblade
Copy link

Blaisorblade commented Aug 21, 2018

First reported in scala/scala3#4936; both sealed and final are redundant on object, but final object seems pretty common (though it's redundant).

scala> trait A { object A }
defined trait A

scala> trait B extends A { object A }
<console>:12: error: overriding object A in trait A;
 object A cannot override final member
       trait B extends A { object A }
                                  ^

OTOH, final object seems pretty common, and it even appears in the new collections, maybe because it is sometimes used in such pattern (from Dotty) — and final on case classes does make sense.

sealed trait Status
final case class Success(output: String) extends Status
final case class Failure(output: String) extends Status
final case object Timeout extends Status

We're considering changing this in Dotty in scala/scala3#4973, but it'd be good to do the change in 2.14 as well I guess — or (EDIT) at least agree that we want to do it.

@SethTisue
Copy link
Member

Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Jan 15, 2019
Fix scala#4936.

- Stop using final object in our sources.
- Give error for abstract and sealed objects, warning for final ones (as long as
- scala/bug#11094 is open).
Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Jan 15, 2019
Fix scala#4936.

- Stop using final object in our sources.
- Give error for abstract and sealed objects, warning for final ones (as long as
- scala/bug#11094 is open).
Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Jan 16, 2019
Fix scala#4936 in parser.

- Stop using final object in our sources.
- Give error for abstract and sealed objects, warning for final ones (as long as
- scala/bug#11094 is open).
- Address review comments.
Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Jan 16, 2019
Fix scala#4936 in parser.

- Stop using final object in our sources.
- Give error for abstract and sealed objects, warning for final ones (as long as
- scala/bug#11094 is open).
- Address review comments.
@Blaisorblade
Copy link
Author

@nrinaudo asks on Gitter:

@SethTisue and @Blaisorblade , here's a question for you (related to your #11094): if final object is going to be forbidden in 2.14, will all objects be flagged as final in the bytecode, or will we still have the current weirdness that case objects in a non-final class are non final in 2.12 (when they were in 2.11)?

Wasn't aware. Is that so???
In Dotty objects are always final, hence the flag is redundant, hence we now have a warning (the flag was used often enough that an error sounded too harsh).
I don't know good reasons why Scalac would have non-final objects, but I have no insight in any transition issues here — I wasn't aware there was a transition.

@nrinaudo
Copy link

Sure. Take the following code:

class Test {
  case object foo
}

Compiled with 2.12.7, this yields the following decompiled bytecode:

public class Test$foo$ implements scala.Product,scala.Serializable {
 // [...]

On the other hand, the same code using 2.11.8, or the following code in 2.12.7:

class Test {
  final case object foo
}

Both compile to:

public final class Test$foo$ implements scala.Product,scala.Serializable {
  // [...]

@Blaisorblade
Copy link
Author

Thanks! FWIW this is even advice: https://nrinaudo.github.io/scala-best-practices/adts/final_case_objects.html.

So, the last Dotty release gets this right. (Somebody should add a test to ensure that stays the case). I won't speculate on how hard changing this will be in Scalac 2.14, if 2.13 doesn't fix it already.

Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Jan 18, 2019
Scalac sometimes *forgets* the final bytecode flag, and sometimes one cares, so
people even *recommend* always writing *final object* instead of object. That's
bad, since we started warning about it in scala#4973.

scala/bug#11094 (comment)
https://nrinaudo.github.io/scala-best-practices/adts/final_case_objects.html

Why care?
=========

If we forget the final flag in bytecode, Java code might then extend the class.
Performance might or might not be affected - some in scala/contributors
debated this for ~20 minutes (starting at
https://gitter.im/scala/contributors?at=5c423c449bfa375aab21f2af).
@dwijnand
Copy link
Member

dwijnand commented Jan 18, 2019

I don't know good reasons why Scalac would have non-final objects, but I have no insight in any transition issues here — I wasn't aware there was a transition.

Because of -Yoverride-objects?

(edit: oh, mentioned in Seth's SO)

@Blaisorblade
Copy link
Author

Happy it’s gone in 2.13. Since Dotty can’t shadow classes, it can’t do that either.

Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Jan 19, 2019
Scalac sometimes *forgets* the final bytecode flag, and sometimes one cares, so
people even *recommend* always writing *final object* instead of object. That's
bad, since we started warning about it in scala#4973.

scala/bug#11094 (comment)
https://nrinaudo.github.io/scala-best-practices/adts/final_case_objects.html

Why care?
=========

If we forget the final flag in bytecode, Java code might then extend the class.
Performance might or might not be affected - some in scala/contributors
debated this for ~20 minutes (starting at
https://gitter.im/scala/contributors?at=5c423c449bfa375aab21f2af).
Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Jan 19, 2019
Scalac sometimes *forgets* the final bytecode flag, and sometimes one cares, so
people even *recommend* always writing *final object* instead of object. That's
bad, since we started warning about it in scala#4973.

scala/bug#11094 (comment)
https://nrinaudo.github.io/scala-best-practices/adts/final_case_objects.html

Why care?
=========

If we forget the final flag in bytecode, Java code might then extend the class.
Performance might or might not be affected - some in scala/contributors
debated this for ~20 minutes (starting at
https://gitter.im/scala/contributors?at=5c423c449bfa375aab21f2af).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants