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

Fix #16405 ctd - wildcards prematurely resolving to Nothing #16764

Merged
merged 2 commits into from
Jan 26, 2023

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jan 25, 2023

Fixes #16405, which was a problem because it could it get in the way of some metaprogramming techniques. The main issue was the fact that when typing functions, the type inference would first look at the types from the source method (in decomposeProtoFunction resolving found type wildcards to Nothing) and only after that, it could look at the target method.

This is a continuation and simplification of #16625

This was a problem because it could it get in the way of some
metaprogramming techniques. The main issue was the fact that when typing
functions, the type inference would first look at the types from the
source method (resolving type wildcards to Nothing) and only after that,
it could look at the target method.
Now, in the case of wildcards we save that fact for later (while still
resolving the prototype parameter to Nothing) and we in that case we
prioritize according to the target method, after which we fallback to
the default procedure.
@odersky
Copy link
Contributor Author

odersky commented Jan 25, 2023

@jchyb It turned out that we don't need to detect wildcards arguments. It's simpler and equally valid to just avoid Nothing as an inferred parameter as long as their is an alternative.

Copy link
Contributor

@jchyb jchyb left a comment

Choose a reason for hiding this comment

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

LGTM! It really does simplify things

@@ -9,5 +9,5 @@ class C extends Q[?] // error: Type argument must be fully defined

object O {
def m(i: Int): Int = i
val x: Q[_] = m // error: result type of lambda is an underspecified SAM type Q[?]
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously I was worried too much that this approach as a whole would create some unreachable code in typedClosure:

  def typedClosure(tree: untpd.Closure, pt: Type)(using Context): Tree = {
    val env1 = tree.env mapconserve (typed(_))
    val meth1 = typedUnadapted(tree.meth)
    val target =
      if (tree.tpt.isEmpty)
        meth1.tpe.widen match {
          case mt: MethodType =>
            pt.findFunctionType match {
              case pt @ SAMType(sam)
              if !defn.isFunctionType(pt) && mt <:< sam =>
                // SAMs of the form C[?] where C is a class cannot be conversion targets.
                // The resulting class `class $anon extends C[?] {...}` would be illegal,
                // since type arguments to `C`'s super constructor cannot be constructed.
                def isWildcardClassSAM =
                  !pt.classSymbol.is(Trait) && pt.argInfos.exists(_.isInstanceOf[TypeBounds])
                val targetTpe =
                  if isFullyDefined(pt, ForceDegree.all) && !isWildcardClassSAM then
                    pt
                  else if pt.isRef(defn.PartialFunctionClass) then
                    // Replace the underspecified expected type by one based on the closure method type
                    defn.PartialFunctionOf(mt.firstParamTypes.head, mt.resultType)
                  else // Unreachable
                    report.error(em"result type of lambda is an underspecified SAM type $pt", tree.srcPos) // 
                    pt
                TypeTree(targetTpe)

But I guess it is should not be a problem, since the checks in the above make the code future proof.

Copy link
Contributor

@jchyb jchyb left a comment

Choose a reason for hiding this comment

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

Oh no, I just noticed that It looks like there are some related problems in the community-build after all

@odersky
Copy link
Contributor Author

odersky commented Jan 25, 2023

Yes, there's a problem with ZIO. I'll try to find out more what caused it.

@odersky
Copy link
Contributor Author

odersky commented Jan 26, 2023

@jchyb It turned out that one needs to take wildcards into account to make both zio and #16405 pass. Making type inference do the expected thing in all the edge cases is really a minefield. My latest changes essentially come back to your idea, but instead of keeping track of wildcards in a separate data structure, we simply keep the original wildcard bounds in decomposeProto.

Copy link
Contributor

@jchyb jchyb left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you

@odersky odersky merged commit be0844e into scala:main Jan 26, 2023
@odersky odersky deleted the fix-i16405 branch January 26, 2023 11:16
@Kordyjan Kordyjan added this to the 3.3.1 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Argument types are erroneously inferred as Nothing when a method is explicitly eta-expanded as in f(_)
3 participants