From d89fbaaad16b57bcf17d58d0cd62389aa6a77fdf Mon Sep 17 00:00:00 2001 From: Yoonjae Jeon Date: Wed, 1 May 2024 01:03:01 +0900 Subject: [PATCH 1/3] Add warning for synchronized on value classes Co-authored-by: Matt Bovel --- .../dotty/tools/dotc/reporting/ErrorMessageID.scala | 2 +- .../src/dotty/tools/dotc/reporting/messages.scala | 10 +++++----- compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 8 +++++++- tests/neg/i17493.check | 11 +++++++++++ tests/neg/i17493.scala | 5 +++++ 5 files changed, 29 insertions(+), 7 deletions(-) create mode 100644 tests/neg/i17493.check create mode 100644 tests/neg/i17493.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index 33f5dcf1b1f5..f47edc0bf4f1 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -200,7 +200,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe case MatchTypeNoCasesID // errorNumber: 184 case UnimportedAndImportedID // errorNumber: 185 case ImplausiblePatternWarningID // errorNumber: 186 - case SynchronizedCallOnBoxedClassID // errorNumber: 187 + case SynchronizedCallOnValueID // errorNumber: 187 case VarArgsParamCannotBeGivenID // errorNumber: 188 case ExtractorNotFoundID // errorNumber: 189 case PureUnitExpressionID // errorNumber: 190 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 2381bd80babc..9b2b2c682249 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2443,13 +2443,13 @@ class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Cont |you intended.$getClassExtraHint""" } -class SynchronizedCallOnBoxedClass(stat: tpd.Tree)(using Context) - extends Message(SynchronizedCallOnBoxedClassID) { +class SynchronizedCallOnValue(stat: tpd.Tree)(using Context) + extends Message(SynchronizedCallOnValueID) { def kind = MessageKind.PotentialIssue - def msg(using Context) = i"Suspicious ${hl("synchronized")} call on boxed class" + def msg(using Context) = i"Suspicious ${hl("synchronized")} call on value" def explain(using Context) = - i"""|You called the ${hl("synchronized")} method on a boxed primitive. This might not be what - |you intended.""" + i"""|You called the ${hl("synchronized")} method on a boxed primitive or on a class extending ${hl("AnyVal")}. + |This might not be what you intended.""" } class ExtensionNullifiedByMember(method: Symbol, target: Symbol)(using Context) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index cdfd137e5661..308cc49f1943 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -1155,6 +1155,11 @@ object RefChecks { then report.warning(ExtensionNullifiedByMember(sym, target.typeSymbol), sym.srcPos) end checkExtensionMethods + def checkSynchronizedCallOnValue(tree: Tree)(using Context): Unit = + if tree.symbol == defn.Object_synchronized + && ctx.owner.enclosingClass.isValueClass then + report.warning(SynchronizedCallOnValue(tree), tree) + /** Verify that references in the user-defined `@implicitNotFound` message are valid. * (i.e. they refer to a type variable that really occurs in the signature of the annotated symbol.) */ @@ -1326,11 +1331,12 @@ class RefChecks extends MiniPhase { thisPhase => override def transformIdent(tree: Ident)(using Context): Tree = checkAnyRefMethodCall(tree) + checkSynchronizedCallOnValue(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) + report.warning(SynchronizedCallOnValue(tree), tree.srcPos) tree } diff --git a/tests/neg/i17493.check b/tests/neg/i17493.check new file mode 100644 index 000000000000..c9af4b813dab --- /dev/null +++ b/tests/neg/i17493.check @@ -0,0 +1,11 @@ +-- [E187] Potential Issue Warning: tests/neg/i17493.scala:3:10 --------------------------------------------------------- +3 | def g = synchronized { println("hello, world") } // warn + | ^^^^^^^^^^^^ + | Suspicious synchronized call on value + |--------------------------------------------------------------------------------------------------------------------- + | Explanation (enabled by `-explain`) + |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + | You called the synchronized method on a boxed primitive or on a class extending AnyVal. + | This might not be what you intended. + --------------------------------------------------------------------------------------------------------------------- +No warnings can be incurred under -Werror (or -Xfatal-warnings) diff --git a/tests/neg/i17493.scala b/tests/neg/i17493.scala new file mode 100644 index 000000000000..41ed3d116671 --- /dev/null +++ b/tests/neg/i17493.scala @@ -0,0 +1,5 @@ +//> using options -explain -Xfatal-warnings +class A(val s: String) extends AnyVal { + def g = synchronized { println("hello, world") } // warn +} +// nopos-error: No warnings can be incurred under -Werror (or -Xfatal-warnings) From f8b2b2ad632f9ad5aedc7b9e5407f03c7cceca9a Mon Sep 17 00:00:00 2001 From: Yoonjae Jeon Date: Wed, 1 May 2024 01:21:59 +0900 Subject: [PATCH 2/3] Fix check files Co-authored-by: Matt Bovel --- tests/warn/17284.check | 18 +++++++++--------- tests/warn/i17266.check | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/warn/17284.check b/tests/warn/17284.check index 47df2d21e3a7..176bfd736ba7 100644 --- a/tests/warn/17284.check +++ b/tests/warn/17284.check @@ -1,30 +1,30 @@ -- [E187] Potential Issue Warning: tests/warn/17284.scala:4:6 ---------------------------------------------------------- 4 | 451.synchronized {} // warn | ^^^^^^^^^^^^^^^^ - | Suspicious synchronized call on boxed class + | Suspicious synchronized call on value |--------------------------------------------------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | You called the synchronized method on a boxed primitive. This might not be what - | you intended. + | You called the synchronized method on a boxed primitive or on a class extending AnyVal. + | This might not be what you intended. --------------------------------------------------------------------------------------------------------------------- -- [E187] Potential Issue Warning: tests/warn/17284.scala:8:4 ---------------------------------------------------------- 8 | x.synchronized {} // warn | ^^^^^^^^^^^^^^ - | Suspicious synchronized call on boxed class + | Suspicious synchronized call on value |--------------------------------------------------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | You called the synchronized method on a boxed primitive. This might not be what - | you intended. + | You called the synchronized method on a boxed primitive or on a class extending AnyVal. + | This might not be what you intended. --------------------------------------------------------------------------------------------------------------------- -- [E187] Potential Issue Warning: tests/warn/17284.scala:11:7 --------------------------------------------------------- 11 | true.synchronized {} // warn | ^^^^^^^^^^^^^^^^^ - | Suspicious synchronized call on boxed class + | Suspicious synchronized call on value |-------------------------------------------------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | You called the synchronized method on a boxed primitive. This might not be what - | you intended. + | You called the synchronized method on a boxed primitive or on a class extending AnyVal. + | This might not be what you intended. -------------------------------------------------------------------------------------------------------------------- diff --git a/tests/warn/i17266.check b/tests/warn/i17266.check index 716cd531dd0a..1c1496f6f1aa 100644 --- a/tests/warn/i17266.check +++ b/tests/warn/i17266.check @@ -23,12 +23,12 @@ -- [E187] Potential Issue Warning: tests/warn/i17266.scala:22:4 -------------------------------------------------------- 22 | 1.synchronized { // warn | ^^^^^^^^^^^^^^ - | Suspicious synchronized call on boxed class + | Suspicious synchronized call on value |-------------------------------------------------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | You called the synchronized method on a boxed primitive. This might not be what - | you intended. + | You called the synchronized method on a boxed primitive or on a class extending AnyVal. + | This might not be what you intended. -------------------------------------------------------------------------------------------------------------------- -- [E181] Potential Issue Warning: tests/warn/i17266.scala:108:2 ------------------------------------------------------- 108 | wait() // warn From 68c30d1efb51f745ef6994ebac55189584c431df Mon Sep 17 00:00:00 2001 From: Yoonjae Jeon Date: Wed, 1 May 2024 11:30:44 +0900 Subject: [PATCH 3/3] Move the test file to tests/warn directory --- tests/neg/i17493.scala | 5 ----- tests/{neg => warn}/i17493.check | 3 +-- tests/warn/i17493.scala | 4 ++++ 3 files changed, 5 insertions(+), 7 deletions(-) delete mode 100644 tests/neg/i17493.scala rename tests/{neg => warn}/i17493.check (85%) create mode 100644 tests/warn/i17493.scala diff --git a/tests/neg/i17493.scala b/tests/neg/i17493.scala deleted file mode 100644 index 41ed3d116671..000000000000 --- a/tests/neg/i17493.scala +++ /dev/null @@ -1,5 +0,0 @@ -//> using options -explain -Xfatal-warnings -class A(val s: String) extends AnyVal { - def g = synchronized { println("hello, world") } // warn -} -// nopos-error: No warnings can be incurred under -Werror (or -Xfatal-warnings) diff --git a/tests/neg/i17493.check b/tests/warn/i17493.check similarity index 85% rename from tests/neg/i17493.check rename to tests/warn/i17493.check index c9af4b813dab..f4ca889ded3c 100644 --- a/tests/neg/i17493.check +++ b/tests/warn/i17493.check @@ -1,4 +1,4 @@ --- [E187] Potential Issue Warning: tests/neg/i17493.scala:3:10 --------------------------------------------------------- +-- [E187] Potential Issue Warning: tests/warn/i17493.scala:3:10 -------------------------------------------------------- 3 | def g = synchronized { println("hello, world") } // warn | ^^^^^^^^^^^^ | Suspicious synchronized call on value @@ -8,4 +8,3 @@ | You called the synchronized method on a boxed primitive or on a class extending AnyVal. | This might not be what you intended. --------------------------------------------------------------------------------------------------------------------- -No warnings can be incurred under -Werror (or -Xfatal-warnings) diff --git a/tests/warn/i17493.scala b/tests/warn/i17493.scala new file mode 100644 index 000000000000..c5bc71a52013 --- /dev/null +++ b/tests/warn/i17493.scala @@ -0,0 +1,4 @@ +//> using options -explain +class A(val s: String) extends AnyVal { + def g = synchronized { println("hello, world") } // warn +}