Skip to content
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

Capture the kse3 issue in test cases and close it #21260

Merged
merged 2 commits into from
Aug 22, 2024
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
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
}
Loading