Fix #4936: Forbid (or warn on) redundant/illegal object flags #4973
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
could be done in the parser no? |
Blaisorblade
force-pushed
the
fix-4936-forbidden-object-flags
branch
from
1ecf67b to
91b540c
Compare
|
@Blaisorblade Inactive for almost 5 months. Can you either finish the PR or close it and re-assign the issue to someone else? Thanks |
Blaisorblade
force-pushed
the
fix-4936-forbidden-object-flags
branch
from
6dcd45e to
debfde5
Compare
Blaisorblade
commented
Jan 15, 2019
| @@ -1 +0,0 @@ | |||
| sealed object Fun | |||
There was a problem hiding this comment.
This testcase was added in #3486 for #3471 to ensure the compiler didn't crash.
During the discussion, it was demanded that final be handled at the same time, which is done here: #3486 (comment).
Blaisorblade
changed the title
Blaisorblade
changed the title
Blaisorblade
added
area:reporting
Blaisorblade
assigned OlivierBlanvillain and nicolasstucki and unassigned Blaisorblade and OlivierBlanvillain
Blaisorblade
force-pushed
the
fix-4936-forbidden-object-flags
branch
from
debfde5 to
0e6095b
Compare
|
That comment does apply, and I guess it overrides contrary suggestions above. |
|
On hold, waiting for decision on flags in #5525. |
Blaisorblade
force-pushed
the
fix-4936-forbidden-object-flags
branch
from
19ce196 to
2b6ec51
Compare
|
FWIW, the current CI failure is spurious; while this is on hold, I'd like to get it working as far as possible. |
Blaisorblade
force-pushed
the
fix-4936-forbidden-object-flags
branch
from
799c6c2 to
d547ea4
Compare
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
force-pushed
the
fix-4936-forbidden-object-flags
branch
from
d547ea4 to
a17b0b6
Compare
|
@nicolasstucki I've done that change, so it could be merged, but @odersky in #5525 suggested centralizing flag checking. Assigning to you for decision. |
|
We can centralize them later in #5525. |
|
I am afraid this is the wrong fix. See our long discussion with Allan. Flag combinations should not be tested in Parser, we discussed this and decided on it. Please let's revert these commits and do it from the ground up at the same time as #5525. |
Blaisorblade
added a commit
to dotty-staging/dotty
that referenced
this pull request
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).
Blaisorblade
added a commit
to dotty-staging/dotty
that referenced
this pull request
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 pull request
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Forbid/warn on redundant/illegal object flags — fix #4936.
Forbid
final objectbug#11094 is open).