Skip to content

Commit

Permalink
Restrict captureWildcards to only be used if needed (#16799)
Browse files Browse the repository at this point in the history
Rather than blindly using the newly wildcard-captured type, check that
it's compatible with the proto/formal type.  That way values that have
wildcard types can be passed, uncast, to extension methods that don't
require the capture.

For instance in specs2, a value of type `Class[? <: Foo]` needn't become
`Class[?1.CAP]` just so it can be applied to `def theValue[T](t: => T)`.

For the zio-http case, despite knowing that JFuture is morally
covariant, we don't have any way to knowing that - so we must be safe
and error.
  • Loading branch information
odersky authored Feb 4, 2023
2 parents 3ef1e73 + 11854bb commit a356581
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 12 deletions.
9 changes: 7 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -721,8 +721,8 @@ trait Applications extends Compatibility {
|| argMatch == ArgMatch.CompatibleCAP
&& {
val argtpe1 = argtpe.widen
val captured = captureWildcards(argtpe1)
(captured ne argtpe1) && isCompatible(captured, formal.widenExpr)
val captured = captureWildcardsCompat(argtpe1, formal.widenExpr)
captured ne argtpe1
}

/** The type of the given argument */
Expand Down Expand Up @@ -2422,4 +2422,9 @@ trait Applications extends Compatibility {
def isApplicableExtensionMethod(methodRef: TermRef, receiverType: Type)(using Context): Boolean =
methodRef.symbol.is(ExtensionMethod) && !receiverType.isBottomType &&
tryApplyingExtensionMethod(methodRef, nullLiteral.asInstance(receiverType)).nonEmpty

def captureWildcardsCompat(tp: Type, pt: Type)(using Context): Type =
val captured = captureWildcards(tp)
if (captured ne tp) && isCompatible(captured, pt) then captured
else tp
}
16 changes: 7 additions & 9 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -501,15 +501,13 @@ object ProtoTypes {

def checkNoWildcardCaptureForCBN(targ1: Tree)(using Context): Tree = {
if hasCaptureConversionArg(targ1.tpe) then
stripCast(targ1).tpe match
case tp: AppliedType if tp.hasWildcardArg =>
errorTree(targ1,
em"""argument for by-name parameter is not a value
|and contains wildcard arguments: $tp
|
|Assign it to a val and pass that instead.
|""")
case _ => targ1
val tp = stripCast(targ1).tpe
errorTree(targ1,
em"""argument for by-name parameter is not a value
|and contains wildcard arguments: $tp
|
|Assign it to a val and pass that instead.
|""")
else targ1
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3962,7 +3962,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
return adaptConstant(tree, ConstantType(converted))
case _ =>

val captured = captureWildcards(wtp)
val captured = captureWildcardsCompat(wtp, pt)
if (captured `ne` wtp)
return readapt(tree.cast(captured))

Expand Down
18 changes: 18 additions & 0 deletions tests/neg/t9419.zio-http.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Minimisation of how the fix for t9419 affected zio-http
import java.util.concurrent.Future as JFuture

trait Test:
def shutdownGracefully(): JFuture[_]

def executedWildcard(jFuture: => JFuture[_]): Unit
def executedGeneric[A](jFuture: => JFuture[A]): Unit
def executedWildGen[A](jFuture: => JFuture[? <: A]): Unit

// Even though JFuture is morally covariant, at least currently,
// there's no definition-side variance, so it's treated as invariant.
// So we have to be concerned that two different values of `JFuture[A]`
// with different types, blowing up together. So error in `fails`.
def works = executedWildcard(shutdownGracefully())
def fails = executedGeneric(shutdownGracefully()) // error
def fixed = executedGeneric(shutdownGracefully().asInstanceOf[JFuture[Any]]) // fix
def best2 = executedWildGen(shutdownGracefully()) // even better, use use-site variance in the method
13 changes: 13 additions & 0 deletions tests/pos/t9419.specs2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Minimisation of how the fix for t9419 affected specs2
class MustExpectable[T](tm: () => T):
def must_==(other: => Any) = tm() == other

class Foo

object Main:
implicit def theValue[T](t: => T): MustExpectable[T] = new MustExpectable(() => t)
def main(args: Array[String]): Unit =
val cls = classOf[Foo]
val instance = new Foo()
val works = cls must_== cls
val fails = instance.getClass must_== cls

0 comments on commit a356581

Please sign in to comment.