Skip to content

Commit

Permalink
Capture the kse3 issue in test cases and close it (#21260)
Browse files Browse the repository at this point in the history
  • Loading branch information
dwijnand authored Aug 22, 2024
2 parents 8b911ef + 6c86910 commit afcb0ad
Show file tree
Hide file tree
Showing 10 changed files with 180 additions and 1 deletion.
17 changes: 16 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,22 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
val qual1 = qual.cast(liftedTp)
val tree1 = cpy.Select(tree0)(qual1, selName)
val rawType1 = selectionType(tree1, qual1)
tryType(tree1, qual1, rawType1)
val adapted = tryType(tree1, qual1, rawType1)
if !adapted.isEmpty && sourceVersion == `3.6-migration` then
val adaptedOld = tryExt(tree, qual)
if !adaptedOld.isEmpty then
val symOld = adaptedOld.symbol
val underlying = liftedTp match
case tp: TypeProxy => i" ${tp.translucentSuperType}"
case _ => ""
report.migrationWarning(
em"""Previously this selected the extension ${symOld}${symOld.showExtendedLocation}
|Now it selects $selName on the opaque type's underlying type$underlying
|
|You can change this back by selecting $adaptedOld
|Or by defining the extension method outside of the opaque type's scope.
|""", tree0)
adapted
else EmptyTree

// Otherwise, try to expand a named tuple selection
Expand Down
7 changes: 7 additions & 0 deletions tests/neg/i21239.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- [E007] Type Mismatch Error: tests/neg/i21239.scala:14:18 ------------------------------------------------------------
14 | def get2: V = get // error
| ^^^
| Found: AnyRef
| Required: V
|
| longer explanation available when compiling with `-explain`
7 changes: 7 additions & 0 deletions tests/neg/i21239.orig.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- [E007] Type Mismatch Error: tests/neg/i21239.orig.scala:32:8 --------------------------------------------------------
32 | get // error
| ^^^
| Found: AnyRef
| Required: V
|
| longer explanation available when compiling with `-explain`
33 changes: 33 additions & 0 deletions tests/neg/i21239.orig.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 1
// A re-minimisated reproduction of the original issue in kse3
// The one in the issue removes the usage of the package
// in the second extension bundle, which is crucial to
// why my change broke this code
package kse.flow

import java.util.concurrent.atomic.AtomicReference

opaque type Worm[V] = AtomicReference[AnyRef]
object Worm:
val notSetSentinel: AnyRef = new AnyRef {}

extension [V](worm: Worm[V])
inline def wormAsAtomic: AtomicReference[AnyRef] = worm

extension [V](worm: kse.flow.Worm[V])

inline def setIfEmpty(v: => V): Boolean =
var old = worm.wormAsAtomic.get()
if old eq Worm.notSetSentinel then
worm.wormAsAtomic.compareAndSet(old, v.asInstanceOf[AnyRef])
else false

inline def get: V = worm.wormAsAtomic.get() match
case x if x eq Worm.notSetSentinel => throw new java.lang.IllegalStateException("Retrieved value before being set")
case x => x.asInstanceOf[V]

inline def getOrSet(v: => V): V = worm.wormAsAtomic.get() match
case x if x eq Worm.notSetSentinel =>
setIfEmpty(v)
get // error
case x => x.asInstanceOf[V]
14 changes: 14 additions & 0 deletions tests/neg/i21239.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// 2
// A more minimised reproduction
package lib

import java.util.concurrent.atomic.AtomicReference

opaque type Worm[V] = AtomicReference[AnyRef]
object Worm:
extension [V](worm: Worm[V])
inline def wormAsAtomic: AtomicReference[AnyRef] = worm

extension [V](worm: lib.Worm[V])
def get: V = worm.wormAsAtomic.get().asInstanceOf[V]
def get2: V = get // error
28 changes: 28 additions & 0 deletions tests/pos/i21239.alt.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// 4
// An alternative way to fix it,
// defining the extension method externally,
// in a scope that doesn't see through
// the opaque type definition.
// The setup here also makes sure those extension
// are on the opaque type's companion object
// (via class extension), meaning that they continue
// to be in implicit scope (as enforced by the usage test)
import java.util.concurrent.atomic.AtomicReference

package lib:
object Worms:
opaque type Worm[V] = AtomicReference[AnyRef]
object Worm extends WormOps:
extension [V](worm: Worm[V])
inline def wormAsAtomic: AtomicReference[AnyRef] = worm

import Worms.Worm
trait WormOps:
extension [V](worm: Worm[V])
def get: V = worm.wormAsAtomic.get().asInstanceOf[V]
def get2: V = get

package test:
import lib.Worms.Worm
object Test:
def usage(worm: Worm[String]): String = worm.get2
34 changes: 34 additions & 0 deletions tests/pos/i21239.orig.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// 5
// Finally, an alternative way to fix the original issue,
// by reimplementing `getOrSet` to not even need
// our `get` extension.
import java.util.concurrent.atomic.AtomicReference

opaque type Worm[V] = AtomicReference[AnyRef]
object Worm:
val notSetSentinel: AnyRef = new AnyRef {}

extension [V](worm: Worm[V])
inline def wormAsAtomic: AtomicReference[AnyRef] = worm // deprecate?

inline def setIfEmpty(v: => V): Boolean =
val x = worm.get()
if x eq notSetSentinel then
val value = v
worm.set(value.asInstanceOf[AnyRef])
true
else false

inline def get: V =
val x = worm.get()
if x eq notSetSentinel then
throw IllegalStateException("Retrieved value before being set")
else x.asInstanceOf[V]

inline def getOrSet(v: => V): V =
val x = worm.get()
if x eq notSetSentinel then
val value = v
worm.set(value.asInstanceOf[AnyRef])
value
else x.asInstanceOf[V]
18 changes: 18 additions & 0 deletions tests/pos/i21239.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 3
// One way to fix the issue, using the
// "universal function call syntax"
// (to borrow from what Rust calls the syntax to
// disambiguate which trait's method is intended.)
import java.util.concurrent.atomic.AtomicReference

package lib:
opaque type Worm[V] = AtomicReference[AnyRef]
object Worm:
extension [V](worm: Worm[V])
def get: V = worm.get().asInstanceOf[V]
def get2: V = Worm.get(worm)

package test:
import lib.Worm
object Test:
def usage(worm: Worm[String]): String = worm.get2
8 changes: 8 additions & 0 deletions tests/warn/i21239.Frac.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- Migration Warning: tests/warn/i21239.Frac.scala:14:8 ----------------------------------------------------------------
14 | f + Frac.wrap(((-g.numerator).toLong << 32) | (g.unwrap & 0xFFFFFFFFL)) // warn
| ^^^
| Previously this selected the extension method + in object Frac
| Now it selects + on the opaque type's underlying type Long
|
| You can change this back by selecting kse.maths.Frac.+(f)
| Or by defining the extension method outside of the opaque type's scope.
15 changes: 15 additions & 0 deletions tests/warn/i21239.Frac.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package kse.maths

import scala.language.`3.6-migration`

opaque type Frac = Long
object Frac {
inline def wrap(f: Long): kse.maths.Frac = f
extension (f: Frac)
inline def unwrap: Long = f
inline def numerator: Int = ((f: Long) >>> 32).toInt
extension (f: kse.maths.Frac)
def +(g: Frac): kse.maths.Frac = f // eliding domain-specific addition logic
def -(g: Frac): kse.maths.Frac =
f + Frac.wrap(((-g.numerator).toLong << 32) | (g.unwrap & 0xFFFFFFFFL)) // warn
}

0 comments on commit afcb0ad

Please sign in to comment.