Skip to content

Warn when calling synchronized on AnyVal #18021

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
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case MatchTypeNoCasesID // errorNumber: 184
case UnimportedAndImportedID // errorNumber: 185
case ImplausiblePatternWarningID // erorNumber: 186
case SynchronizedCallOnBoxedClassID // errorNumber: 187

def errorNumber = ordinal - 1

Expand Down
9 changes: 9 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,15 @@ class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Cont
|you intended."""
}

class SynchronizedCallOnBoxedClass(stat: tpd.Tree)(using Context)
extends Message(SynchronizedCallOnBoxedClassID) {
def kind = MessageKind.PotentialIssue
def msg(using Context) = i"Suspicious ${hl("synchronized")} call on boxed class"
def explain(using Context) =
i"""|You called the ${hl("synchronized")} method on a boxed primitive. This might not be what
|you intended."""
}

class TraitCompanionWithMutableStatic()(using Context)
extends SyntaxMsg(TraitCompanionWithMutableStaticID) {
def msg(using Context) = i"Companion of traits cannot define mutable @static fields"
Expand Down
4 changes: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,10 @@ class RefChecks extends MiniPhase { thisPhase =>
checkAnyRefMethodCall(tree)
tree

override def transformSelect(tree: tpd.Select)(using Context): tpd.Tree =
if defn.ScalaBoxedClasses().contains(tree.qualifier.tpe.typeSymbol) && tree.name == nme.synchronized_ then
report.warning(SynchronizedCallOnBoxedClass(tree), tree.srcPos)
tree
}

/* todo: rewrite and re-enable
Expand Down
30 changes: 30 additions & 0 deletions tests/neg/17284.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
-- [E187] Potential Issue Error: tests/neg/17284.scala:4:6 -------------------------------------------------------------
4 | 451.synchronized {} // error
| ^^^^^^^^^^^^^^^^
| Suspicious synchronized call on boxed class
|---------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| You called the synchronized method on a boxed primitive. This might not be what
| you intended.
---------------------------------------------------------------------------------------------------------------------
-- [E187] Potential Issue Error: tests/neg/17284.scala:8:4 -------------------------------------------------------------
8 | x.synchronized {} // error
| ^^^^^^^^^^^^^^
| Suspicious synchronized call on boxed class
|---------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| You called the synchronized method on a boxed primitive. This might not be what
| you intended.
---------------------------------------------------------------------------------------------------------------------
-- [E187] Potential Issue Error: tests/neg/17284.scala:11:7 ------------------------------------------------------------
11 | true.synchronized {} // error
| ^^^^^^^^^^^^^^^^^
| Suspicious synchronized call on boxed class
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| You called the synchronized method on a boxed primitive. This might not be what
| you intended.
--------------------------------------------------------------------------------------------------------------------
14 changes: 14 additions & 0 deletions tests/neg/17284.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// scalac: -Werror -explain

def test =
451.synchronized {} // error

def test2 =
val x: Integer = 451
x.synchronized {} // error

def test3 =
true.synchronized {} // error

def test4 =
true.hashCode() // success
10 changes: 10 additions & 0 deletions tests/neg/i17266.check
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@
| resolved to calls on Predef or on imported methods. This might not be what
| you intended.
--------------------------------------------------------------------------------------------------------------------
-- [E187] Potential Issue Error: tests/neg/i17266.scala:22:4 -----------------------------------------------------------
22 | 1.synchronized { // error
| ^^^^^^^^^^^^^^
| Suspicious synchronized call on boxed class
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| You called the synchronized method on a boxed primitive. This might not be what
| you intended.
--------------------------------------------------------------------------------------------------------------------
-- [E181] Potential Issue Error: tests/neg/i17266.scala:108:2 ----------------------------------------------------------
108 | wait() // error
| ^^^^
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/i17266.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def test3 =
}

def test4 =
1.synchronized { // not an error (should be?)
1.synchronized { // error
println("hello")
}

Expand Down