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

Don't retypecheck erroneous arguments when fixing function #14043

Merged
merged 7 commits into from
Jan 24, 2022
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
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
odersky marked this conversation as resolved.
Show resolved Hide resolved
// 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