Should Generate warnings when synchronized on `AnyVal`

[He-Pin]

Compiler version

3.2.2

Minimized code

1.synchronized{}

Output

Expectation

generate some warning as https://openjdk.org/jeps/390

[liang3zy22]

This issue also exists with scala 2.13.10.

[som-snytt]

That is due to boxing conversion.

g does not compile in Scala 2, but in Scala 3 is accepted, as though written A.synchronized.

class A(val s: String) extends AnyVal {
  //def f = eq("hello, world")  // missing argument list for method eq in object Predef

  def g = synchronized { println("hello, world") }
}

The error on f is charming. I was expecting it to use (Predef: AnyRef).eq and was hoping for a warning.

Since this ticket is an enhancement, I created a separate issue for the missing error.

[scala-center-bot]

This issue was picked for the Issue Spree No. 32 of 20 June 2023 which takes place in 7 days. @mbovel, @diogocanut, @jmesyou, @michaelpigg will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

[mbovel]

See synchronized method documentation.

[mbovel]

I don't know good ressources about autoboxing in Scala, but the one for Java might be worth reading: https://docs.oracle.com/javase/tutorial/java/data/autoboxing.html.

[michaelpigg]

It's not clear to me what phase of the compiler is relevant to this issue. I was thinking initially the parser, but maybe it's at erasure after the tree shows it converted to a BoxedUnit?

[mbovel]

@michaelpigg, sorry we missed you for this spree! The phase where we added the check is RefChecks: https://github.com/lampepfl/dotty/blob/main/compiler/src/dotty/tools/dotc/typer/RefChecks.scala. This phase runs after the parser and the typer, but before erasure. It's where we have most checks that are independent of parsing and typing.

To investigate this issue, we first created a test file at tests/neg/17284.scala with the following content:

def test = 451.synchronized {}

And then printed the AST after the typer using scalac -Xprint:typer tests/neg/17284.scala:

[[syntax trees at end of                     typer]] // tests/neg/17284.scala
package <empty> {
  final lazy module val 17284$package: 17284$package = new 17284$package()
  final module class 17284$package() extends Object() {
    this: 17284$package.type =>
    def test: Unit =
      int2Integer(451).synchronized[Unit](
        {
          ()
        }
      )
  }
}

We also used the -Yplain-printer flag to print the actual tree structure, not the pretty printed version scalac -Xprint:typer -Yplain-printer tests/neg/17284.scala:

[[syntax trees at end of                     typer]] // tests/neg/17284.scala
PackageDef(Ident(<empty>), List(
  ValDef(17284$package, Ident(17284$package$),
    Apply(Select(New(Ident(17284$package$)), <init>), List())),
  TypeDef(17284$package$,
    Template(DefDef(<init>, List(List()), TypeTree(), Thicket(List())),
      List(Apply(Select(New(TypeTree()), <init>), List())),
      ValDef(_, SingletonTypeTree(Ident(17284$package)), Thicket(List())), List(
      DefDef(test, List(), TypeTree(),
        Apply(
          TypeApply(
            Select(Apply(Ident(int2Integer), List(Literal(451))), synchronized)
              ,
          List(TypeTree())),
        List(Block(List(), Literal(()))))
      )
    ))
  )
))

And noted that the part we are interested in is Select(Apply(Ident(int2Integer), List(Literal(451))), synchronized).

More specifically, we're looking for pattern of the form Select(qualifier, name) where qualifier's type is a boxed class, and name is synchronized.

You can find the result of our work in #18021.

Don't hesitate to ask if you have questions! I hope to see you at the next Spree.

[michaelpigg]

Thanks for the explanation, @mbovel !