TypedTreeCopier makes incorrect assumption when deciding if Block or If nodes need to be retyped

[smarter]

For blocks, from tpd.scala:

tree match {
  case tree: Block if (expr.tpe eq tree.expr.tpe) => tree1.withTypeUnchecked(tree.tpe)
  case _ => ta.assignType(tree1, stats, expr)
}

This assumes that tree.tpe and tree.expr.tpe are in sync, but with a block like:

{
  val y: Position = new Position(1)
  y
}

Then tree.expr.tpe will be a TermRef but tree.tpe will be a widened version of that type since the TermRef should not escape the scope of the block. This means that if the info of y is changed by DenotTransformer#transform:

• tree.expr.tpe will still be a TermRef
• tree.expr.tpe.widen will be changed
• tree.tpe will not be changed, is now incorrect, and needs to be retyped.

This happens in phase ElimErasedValueType in my implementation of value classes, see https://github.com/smarter/dotty/tree/add/value-classes%2Bcpy-bug and reproduce the bug by running:

> run -Ycheck:elimErasedValueType tests/pos/vcblock.scala
> run -Ycheck:elimErasedValueType tests/pos/vcif.scala

And note that the errors disappear if you always retype Block and If: https://github.com/smarter/dotty/tree/add/value-classes%2Bcpy-bug%2Bretype

Assigned to @DarkDimius .

[DarkDimius]

@smarter, in future, can you please put value-classes-related tests into separate folder? Like the one for tailrec

[smarter]

Sure.

[DarkDimius]

@smarter see PR

[smarter]

@felixmulder Any reason this was closed? The PR was not applied, though I don't remember why we decided to not merge it at the time

[felixmulder]

I'm not sure why this was closed - maybe I thought the fix was merged (or maybe tool magic). Could we get a comment on why the fix wasn't merged @DarkDimius ?