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

SI-7944 FOUND: stray undetermined type params in vicinity of implicits #3088

Merged
merged 1 commit into from Nov 3, 2013

Conversation

retronym
Copy link
Member

Implicit search created a nested Context into which the results of
its typechecking, namely, errors and undetermined type parameters
(roughly: those inferred as Nothing) are stashed.

The code the drives the process was checking for errors, but
discarded those undetermined type parameters.

This commit copies them from the child context to the parent,
which lets Typer#adapt to get to:

else if (hasUndetsInMonoMode) { // (9)
  assert(!context.inTypeConstructorAllowed, context) //@M
  instantiatePossiblyExpectingUnit(tree, mode, pt)
}

Our lost TypeVar has found its way home! The reward for which
is being instantiated, based on another type inference session
adapting the expression's type to the expected type.

@retronym
Copy link
Member Author

Review by @adriaanm

@adriaanm
Copy link
Contributor

And I didn't even have to go around and put up MISSING posters!

// `implicitSearchContext.undetparams`, *not* in `context.undetparams`
// Here, we copy them up to parent context (analogously to the way the errors are copied above),
// and then filter out any which *were* inferred and are part of the substitutor in the implicit search result.
context.undetparams = ((context.undetparams ++ implicitSearchContext.undetparams) filterNot result.subst.from.contains).distinct
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer. I traced this through to better understand the process.

By way of example, in:

scala> val ls = List[Option[Any]]()
ls: List[Option[Any]] = List()

scala> ls.flatten

The undetermined type params from the outer context are B(in method flatten), during the implicit search for the implicit parameter of type Option[Any] => scala.collection.GenTraversableOnce[B].

That is first used to approximate the sought type as Option[Any] => scala.collection.GenTraversableOnce[?] (wildPt) which is used to prune the candidate implicits.

Later, when typechecking the plausible candidates, occurrences of the undet params are replaced by type variables in the expected type, and matchesPt is called to see if the candidate is suitable:

itree2 = {
  ((xo: Option[Any]) => scala.this.Option.option2Iterable[Any](xo))
}: Option[Any] => Iterable[Any]

matchesPt(
  tp = Option[Any] => Iterable[Any],
  pt = Option[Any] => scala.collection.GenTraversableOnce[?B]
  undet = [type B]
)

What does matchesPt do with the undet? Ultimately, they are used in isApplicableToMethod:

      def typesCompatible(args: List[Type]) = undetparams match {
        case Nil => isCompatibleArgs(args, formals) && isWeaklyCompatible(mt resultType args, pt)
        case _   => tryInstantiating(args)
      }

      def tryInstantiating(args: List[Type]) = falseIfNoInstance {
        val restpe = mt resultType args
        val AdjustedTypeArgs.Undets(okparams, okargs, leftUndet) = methTypeArgs(undetparams, formals, restpe, args, pt)
        val restpeInst = restpe.instantiateTypeParams(okparams, okargs)

@retronym
Copy link
Member Author

@cunei @luc @harrah

Does this failure look familiar?

[info] [error] /localhome/jenkins/b/workspace/pr-scala-integrate-ide/target/zinc/target-0.6.4-josh-hack-3/project-builds/zinc-77374c3a564c525ae9145677ec14c0bae74af3d3/src/main/scala/com/typesafe/zinc/SbtAnalysis.scala:58: value merge is not a member of object sbt.inc.Analysis
[info] [error]     val mergedAnalysis = Analysis.merge(analysesAndSetups map {_._1})
[info] [error]                                   ^
[info] [error] /localhome/jenkins/b/workspace/pr-scala-integrate-ide/target/zinc/target-0.6.4-josh-hack-3/project-builds/zinc-77374c3a564c525ae9145677ec14c0bae74af3d3/src/main/scala/com/typesafe/zinc/SbtAnalysis.scala:58: value merge is not a member of object sbt.inc.Analysis
[info] [error]     val mergedAnalysis = Analysis.merge(analysesAndSetups map {_._1})

The method reported as missing was recently added to SBT:

sbt/sbt@30cdb1a

@gkossakowski
Copy link
Member

Zinc master has been updated (typesafehub/zinc@54f5665) to use new apis introduced in sbt/sbt@30cdb1a but in PR validation zinc master is compiled against stable release of sbt.

I believe we should either compile stable versions of everything or masters of everything. I'd prefer to compile everything from stable versions (tags).

@gkossakowski
Copy link
Member

Actually, it uses a bit old branch: https://github.com/typesafehub/sbt-builds-for-ide/blob/master/sbt-on-2.11.x#L56
I'll issue a PR which updates that branch to sbt 0.13.
Also, there'll be another PR which uses stable (0.3) version of zinc for building.

@gkossakowski
Copy link
Member

However, I can do it only tomorrow's evening. Feel free to beat on this.

@adriaanm
Copy link
Contributor

I'm working on the sbt build for #3087, so I'll take this. (Does the 0.13 branch build as-is on 2.11 these days?)

Implicit search created a nested Context into which the results of
its typechecking, namely, errors and undetermined type parameters
(roughly: those inferred as Nothing) are stashed.

The code the drives the process was checking for errors, but
discarded those undetermined type parameters.

This commit copies them from the child context to the parent,
which lets `Typer#adapt` to get to:

    else if (hasUndetsInMonoMode) { // (9)
      assert(!context.inTypeConstructorAllowed, context) //@m
      instantiatePossiblyExpectingUnit(tree, mode, pt)
    }

Our lost TypeVar has found its way home! The reward for which
is being instantiated, based on another type inference session
adapting the expression's type to the expected type.
@retronym
Copy link
Member Author

The latest force push just expands the test case a little.

@ghost ghost assigned adriaanm Oct 31, 2013
@adriaanm
Copy link
Contributor

adriaanm commented Nov 3, 2013

LGTM

retronym added a commit that referenced this pull request Nov 3, 2013
SI-7944 FOUND: stray undetermined type params in vicinity of implicits
@retronym retronym merged commit 4a6882e into scala:master Nov 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants