Skip to content

Commit

Permalink
Merge pull request #14043 from dotty-staging/fix-12941
Browse files Browse the repository at this point in the history
Don't retypecheck erroneous arguments when fixing function
  • Loading branch information
nicolasstucki authored Jan 24, 2022
2 parents 22b9b01 + 89ec460 commit 0e4e5d5
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 7 deletions.
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,9 @@ trait Applications extends Compatibility {
* part. Return an optional value to indicate success.
*/
def tryWithImplicitOnQualifier(fun1: Tree, proto: FunProto)(using Context): Option[Tree] =
if (ctx.mode.is(Mode.SynthesizeExtMethodReceiver))
if ctx.mode.is(Mode.SynthesizeExtMethodReceiver) || proto.hasErrorArg then
// Suppress insertion of apply or implicit conversion on extension method receiver
// or if argument is erroneous by itself.
None
else
tryInsertImplicitOnQualifier(fun1, proto, ctx.typerState.ownedVars) flatMap { fun2 =>
Expand Down
35 changes: 33 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import Contexts._, Types._, Denotations._, Names._, StdNames._, NameOps._, Symbo
import NameKinds.DepParamName
import Trees._
import Constants._
import util.{Stats, SimpleIdentityMap}
import util.{Stats, SimpleIdentityMap, SimpleIdentitySet}
import Decorators._
import Uniques._
import config.Printers.typr
Expand Down Expand Up @@ -282,6 +282,9 @@ object ProtoTypes {
/** A map in which typed arguments can be stored to be later integrated in `typedArgs`. */
var typedArg: SimpleIdentityMap[untpd.Tree, Tree] = SimpleIdentityMap.empty

/** The argument that produced errors during typing */
var errorArgs: SimpleIdentitySet[untpd.Tree] = SimpleIdentitySet.empty

/** The tupled or untupled version of this prototype, if it has been computed */
var tupledDual: Type = NoType

Expand Down Expand Up @@ -341,6 +344,30 @@ object ProtoTypes {
case _ => false
}

/** Did an argument produce an error when typing? This means: an error was reported
* and a tree got an error type. Errors of adaptation whree a tree has a good type
* but that type does not conform to the expected type are not counted.
*/
def hasErrorArg = !state.errorArgs.isEmpty

/** Does tree have embedded error trees that are not at the outside.
* A nested tree t1 is "at the outside" relative to a tree t2 if
* - t1 and t2 have the same span, or
* - t2 is a ascription (t22: T) and t1 is at the outside of t22
* - t2 is a closure (...) => t22 and t1 is at the outside of t22
*/
def hasInnerErrors(t: Tree): Boolean = t match
case Typed(expr, tpe) => hasInnerErrors(expr)
case closureDef(mdef) => hasInnerErrors(mdef.rhs)
case _ =>
t.existsSubTree { t1 =>
if t1.typeOpt.isError && t1.span.toSynthetic != t.span.toSynthetic then
typr.println(i"error subtree $t1 of $t with ${t1.typeOpt}, spans = ${t1.span}, ${t.span}")
true
else
false
}

private def cacheTypedArg(arg: untpd.Tree, typerFn: untpd.Tree => Tree, force: Boolean)(using Context): Tree = {
var targ = state.typedArg(arg)
if (targ == null)
Expand All @@ -357,8 +384,12 @@ object ProtoTypes {
targ = arg.withType(WildcardType)
case _ =>
targ = typerFn(arg)
if (!ctx.reporter.hasUnreportedErrors)
if ctx.reporter.hasUnreportedErrors then
if hasInnerErrors(targ) then
state.errorArgs += arg
else
state.typedArg = state.typedArg.updated(arg, targ)
state.errorArgs -= arg
}
targ
}
Expand Down
16 changes: 16 additions & 0 deletions tests/neg-custom-args/deprecation/t3235-minimal.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
-- Error: tests/neg-custom-args/deprecation/t3235-minimal.scala:3:21 ---------------------------------------------------
3 | assert(123456789.round == 123456789) // error
| ^^^^^^^^^^^^^^^
|method round in class RichInt is deprecated since 2.11.0: this is an integer type; there is no reason to round it. Perhaps you meant to call this on a floating-point value?
-- Error: tests/neg-custom-args/deprecation/t3235-minimal.scala:4:16 ---------------------------------------------------
4 | assert(math.round(123456789) == 123456789) // error
| ^^^^^^^^^^
|method round in package scala.math is deprecated since 2.11.0: This is an integer type; there is no reason to round it. Perhaps you meant to call this with a floating-point value?
-- Error: tests/neg-custom-args/deprecation/t3235-minimal.scala:5:32 ---------------------------------------------------
5 | assert(1234567890123456789L.round == 1234567890123456789L) // error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|method round in class RichLong is deprecated since 2.11.0: this is an integer type; there is no reason to round it. Perhaps you meant to call this on a floating-point value?
-- Error: tests/neg-custom-args/deprecation/t3235-minimal.scala:6:16 ---------------------------------------------------
6 | assert(math.round(1234567890123456789L) == 1234567890123456789L) // error
| ^^^^^^^^^^
|method round in package scala.math is deprecated since 2.11.0: This is an integer type; there is no reason to round it. Perhaps you meant to call this with a floating-point value?
28 changes: 28 additions & 0 deletions tests/neg/i12941.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
object A:
def myFun(op: String ?=> Unit) = ()

@main def func: Unit =
A.myFun {
val res: String = summon[String]
println(ress) // error
}

class I:
def runSth: Int = 1

abstract class A:
def myFun(op: I ?=> Unit) =
op(using I())
1

class B extends A

def assertEquals(x: String, y: Int, z: Int): Unit = ()

@main def hello: Unit =

B().myFun {
val res = summon[I].runSth
assertEquals("", 1, res, "asd") // error
println("Hello!")
}
9 changes: 9 additions & 0 deletions tests/neg/zipped.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
object Test {
val xs: List[Int] = ???

xs.lazyZip(xs).lazyZip(xs)
.map( (x: (Int, Int, Int)) => x match { case (x, y, z) => x + y + z }) // ok

xs.lazyZip(xs).lazyZip(xs)
.map( x => x match { case (x, y, z) => x + y + z }) // error
}
20 changes: 20 additions & 0 deletions tests/pos/specs2-failure.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import util.matching.Regex
import util.matching.Regex.Match

// Demonstrate what used to be a failure in specs2, before we refined
// the scheme when not to typecheck a function argument again.
object Test:

extension (s: String)

def replaceAll(pairs: (String, String)*): String =
pairs.foldLeft(s) { (res, cur) =>
res.replaceAll(cur._1, cur._2)
}

def replaceAll(exp: String, f: String => String): String =
new Regex(exp).replaceAllIn(s, (m: Match) => f(m.group(0).replace("\\", "\\\\")))

def replaceInsideTag(tag: String, p: (String, String)*): String =
s.replaceAll(tag, (s: String) => java.util.regex.Matcher.quoteReplacement(s.replaceAll(p*)))

7 changes: 4 additions & 3 deletions tests/pos/zipped.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ object Test {
xs.lazyZip(xs).lazyZip(xs)
.map( (x: (Int, Int, Int)) => x match { case (x, y, z) => x + y + z }) // now also OK

// 5. If we leave out the parameter type, it now works as well.
xs.lazyZip(xs).lazyZip(xs)
.map( x => x match { case (x, y, z) => x + y + z }) // now also OK
// 5. But if that pone is deeper nested, it does not work since we don't retypecheck
// arguments deeply.
//xs.lazyZip(xs).lazyZip(xs)
// .map( x => x match { case (x, y, z) => x + y + z }) // now also OK

// This means that the following works in Dotty 3.0 as well as 3.x
for ((x, y, z) <- xs.lazyZip(xs).lazyZip(xs)) yield x + y + z
Expand Down

0 comments on commit 0e4e5d5

Please sign in to comment.