From bee299e792116f471f12dadf31360c7d208130b3 Mon Sep 17 00:00:00 2001
From: Guillaume Martres <smarter@ubuntu.com>
Date: Thu, 19 Aug 2021 21:28:46 +0200
Subject: [PATCH] wildApprox: also approximate FunProto arguments

Previously, `wildApprox` delegated FunProto handling to
`TypeMap#mapOver` which only maps the result type of the FunProto, but
the result of `wildApprox` can be passed to
`ImplicitRunInfo#computeIScope` which will end up calling
`FunProto#typedArgs`. This caused two problems:
- It forced us to use a `provisional` flag in `computeIScope` to account
  for uninstantiated type variables that might show up in the result of
  typedArgs.
- It lead to an assertion failure (in i13340.scala) because it turns out
  that `typedArgs` doesn't always respect the pre-conditions of
  `mergeConstraintWith` (see added NOTE).

By approximating FunProto arguments in wildApprox we can drop the
`provisional` flag and avoid the issue with `typedArgs` (though we might
need to actually fix `typedArgs` one day if the same issue shows up in
other circumstances).

Fixes #13340.
---
 .../src/dotty/tools/dotc/typer/Implicits.scala   | 11 ++++-------
 .../src/dotty/tools/dotc/typer/ProtoTypes.scala  | 16 +++++++++++++++-
 tests/neg/i13340.scala                           |  3 +++
 3 files changed, 22 insertions(+), 8 deletions(-)
 create mode 100644 tests/neg/i13340.scala

diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala
index d22d70b41bfc..5a224005a7d1 100644
--- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala
@@ -562,7 +562,6 @@ trait ImplicitRunInfo:
 
     object collectParts extends TypeTraverser:
 
-      private var provisional: Boolean = _
       private var parts: mutable.LinkedHashSet[Type] = _
       private val partSeen = util.HashSet[Type]()
 
@@ -587,19 +586,18 @@ trait ImplicitRunInfo:
             case t: ConstantType =>
               traverse(t.underlying)
             case t: TypeParamRef =>
+              assert(!ctx.typerState.constraint.contains(t), i"`wildApprox` failed to remove uninstantiated $t")
               traverse(t.underlying)
-              if ctx.typerState.constraint.contains(t) then provisional = true
             case t: TermParamRef =>
               traverse(t.underlying)
             case t =>
               traverseChildren(t)
 
-      def apply(tp: Type): (collection.Set[Type], Boolean) =
-        provisional = false
+      def apply(tp: Type): collection.Set[Type] =
         parts = mutable.LinkedHashSet()
         partSeen.clear()
         traverse(tp)
-        (parts, provisional)
+        parts
     end collectParts
 
     val seen = util.HashSet[Type]()
@@ -674,12 +672,11 @@ trait ImplicitRunInfo:
     end collectCompanions
 
     def recur(tp: Type): OfTypeImplicits =
-      val (parts, provisional) = collectParts(tp)
+      val parts = collectParts(tp)
       val companions = collectCompanions(tp, parts)
       val result = OfTypeImplicits(tp, companions)(runContext)
       if Config.cacheImplicitScopes
         && tp.hash != NotCached
-        && !provisional
         && (tp eq rootTp)              // first type traversed is always cached
            || !incomplete.contains(tp) // other types are cached if they are not incomplete
       then implicitScopeCache(tp) = result
diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
index fb701c1e2e94..167128be08e4 100644
--- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
+++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
@@ -394,7 +394,10 @@ object ProtoTypes {
             // `protoTyperState` committable we must ensure that it does not
             // contain any type variable which don't already exist in the passed
             // TyperState. This is achieved by instantiating any such type
-            // variable.
+            // variable. NOTE: this does not suffice to discard type variables
+            // in ancestors of `protoTyperState`, if this situation ever
+            // comes up, an assertion in TyperState will trigger and this code
+            // will need to be generalized.
             if protoTyperState.isCommittable then
               val passedConstraint = passedTyperState.constraint
               val newLambdas = newConstraint.domainLambdas.filter(tl =>
@@ -822,6 +825,17 @@ object ProtoTypes {
       tp.derivedViewProto(
           wildApprox(tp.argType, theMap, seen, internal),
           wildApprox(tp.resultType, theMap, seen, internal))
+    case tp: FunProto =>
+      val args = tp.args.mapconserve(arg =>
+        val argTp = tp.typeOfArg(arg) match
+          case NoType => WildcardType
+          case tp => wildApprox(tp, theMap, seen, internal)
+        arg.withType(argTp))
+      val resTp = wildApprox(tp.resultType, theMap, seen, internal)
+      if (args eq tp.args) && (resTp eq tp.resultType) then
+        tp
+      else
+        FunProtoTyped(args, resTp)(ctx.typer, tp.applyKind)
     case tp: IgnoredProto =>
       WildcardType
     case  _: ThisType | _: BoundType => // default case, inlined for speed
diff --git a/tests/neg/i13340.scala b/tests/neg/i13340.scala
new file mode 100644
index 000000000000..1db3f32af52d
--- /dev/null
+++ b/tests/neg/i13340.scala
@@ -0,0 +1,3 @@
+case class Field(name: String, subQuery: Option[Query] = None)
+case class Query(fields: Seq[Field])
+val x = Query(Seq(Field("a", subQuery=Some(Query(Seq(Field("b")), Nil)))), Nil) // error