Skip to content

Test final object is indeed redundant: objects must be JVM-final #5745

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

Merged
merged 1 commit into from
Jan 19, 2019

Conversation

Blaisorblade
Copy link
Contributor

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 #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 Blaisorblade requested a review from smarter January 18, 2019 21:25
@Blaisorblade Blaisorblade added stat:needs review fasttrack Simple fix. Reviewer should merge or apply additional changes directly. labels Jan 18, 2019
@Test def objectsAreFinal = {
val source =
"""class Test {
| object Foo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would test a few more cases (top-level object, object in object, object in local block).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Better?

@smarter smarter assigned Blaisorblade and unassigned smarter Jan 18, 2019
@Blaisorblade Blaisorblade force-pushed the test-objects-are-final branch from 5dcc31d to e239918 Compare January 19, 2019 10:21
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 Blaisorblade force-pushed the test-objects-are-final branch from e239918 to 29e1f8a Compare January 19, 2019 10:22
@Blaisorblade Blaisorblade assigned smarter and unassigned Blaisorblade Jan 19, 2019
@Blaisorblade Blaisorblade merged commit 9b74ac1 into scala:master Jan 19, 2019
@Blaisorblade Blaisorblade deleted the test-objects-are-final branch January 19, 2019 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fasttrack Simple fix. Reviewer should merge or apply additional changes directly.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants