Merge pull request scala#6475 from adriaanm/eta_cleanup

Do not eta-expand 0-arg methods. Improve eta-expansion for method values.

Author: adriaanm

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

@@ -667,14 +667,14 @@ trait ContextErrors {
       def MissingArgsForMethodTpeError(tree: Tree, meth: Symbol) = {
         val f = meth.name.decoded
         val paf = s"$f(${ meth.asMethod.paramLists map (_ map (_ => "_") mkString ",") mkString ")(" })"
-        val advice = s"""
-          |Unapplied methods are only converted to functions when a function type is expected.
-          |You can make this conversion explicit by writing `$f _` or `$paf` instead of `$f`.""".stripMargin
+        val advice =
+          if (meth.isConstructor || meth.info.params.length > definitions.MaxFunctionArity) ""
+          else s"""
+            |Unapplied methods are only converted to functions when a function type is expected.
+            |You can make this conversion explicit by writing `$f _` or `$paf` instead of `$f`.""".stripMargin
         val message =
           if (meth.isMacro) MacroTooFewArgumentListsMessage
-          else s"""missing argument list for ${meth.fullLocationString}${
-            if (!meth.isConstructor) advice else ""
-          }"""
+          else s"""missing argument list for ${meth.fullLocationString}$advice"""
         issueNormalTypeError(tree, message)
         setError(tree)
       }
@@ -730,9 +730,12 @@ trait ContextErrors {
         setError(tree)
       }
 
+      def DependentMethodTpeConversionToFunctionError(tree: Tree, tp: Type): Tree = {
+        issueNormalTypeError(tree, "method with dependent type " + tp + " cannot be converted to function value")
+        setError(tree)
+      }
+
       // cases where we do not necessarily return trees
-      def DependentMethodTpeConversionToFunctionError(tree: Tree, tp: Type) =
-        issueNormalTypeError(tree, "method with dependent type "+tp+" cannot be converted to function value")
 
       //checkStarPatOK
       def StarPatternWithVarargParametersError(tree: Tree) =
@@ -1375,9 +1378,11 @@ trait ContextErrors {
           )
         }
 
+        // Note that treeInfo.Applied always matches, it just returns Nil when no application was found...
         def treeTypeArgs(annotatedTree: Tree): List[String] = annotatedTree match {
-          case TypeApply(_, args) => args.map(_.toString)
           case Block(_, Function(_, treeInfo.Applied(_, targs, _))) => targs.map(_.toString) // eta expansion, see neg/t9527b.scala
+          case Function(_, treeInfo.Applied(_, targs, _)) => targs.map(_.toString) // eta expansion, see neg/t9527b.scala
+          case treeInfo.Applied(_, targs, _) => targs.map(_.toString)
           case _ => Nil
         }
 

src/compiler/scala/tools/nsc/typechecker/EtaExpansion.scala

@@ -17,7 +17,7 @@ import symtab.Flags._
 trait EtaExpansion { self: Analyzer =>
   import global._
 
-  /** Expand partial method application `p.f(es_1)...(es_n)`.
+  /** Expand partial method application `p.f(es_1)...(es_n)`. Does not support dependent method types (yet).
     *
     * We expand this to the following block, which evaluates
     * the target of the application and its supplied arguments if needed (they are not stable),
@@ -33,8 +33,9 @@ trait EtaExpansion { self: Analyzer =>
     * }
     * ```
     *
-    * This is called from instantiateToMethodType after type checking `tree`,
-    * and we realize we have a method type, where a function type (builtin or SAM) is expected.
+    * This is called from typedEtaExpansion, which itself is called from
+    *   - instantiateToMethodType (for a naked method reference), or
+    *   - typedEta (when type checking a method value, `m _`).
     *
     **/
   def etaExpand(unit: CompilationUnit, tree: Tree, typer: Typer): Tree = {
@@ -120,6 +121,9 @@ trait EtaExpansion { self: Analyzer =>
     }
 
     val tree1 = liftoutPrefix(tree)
-    atPos(tree.pos)(Block(defs.toList, expand(tree1, tpe)))
+    val expansion = expand(tree1, tpe)
+
+    if (defs.isEmpty) expansion
+    else atPos(tree.pos)(Block(defs.toList, expansion))
   }
 }

src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala

@@ -205,9 +205,6 @@ trait StdAttachments {
 
   def removeStabilizingDefinitions(tree: Tree): Tree =
     tree.removeAttachment[StabilizingDefinitions](StabilizingDefinitionsTag)
-
-  /** Added to trees that appear in a method value, e.g., to `f(x)` in `f(x) _` */
-  case object MethodValueAttachment
 }
 
 

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -348,24 +348,6 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
       }
     }
 
-    def checkParamsConvertible(tree: Tree, tpe0: Type): Unit = {
-      def checkParamsConvertible0(tpe: Type) =
-        tpe match {
-          case MethodType(formals, restpe) =>
-            /*
-            if (formals.exists(_.typeSymbol == ByNameParamClass) && formals.length != 1)
-              error(pos, "methods with `=>`-parameter can be converted to function values only if they take no other parameters")
-            if (formals exists (isRepeatedParamType(_)))
-              error(pos, "methods with `*`-parameters cannot be converted to function values");
-            */
-            if (tpe.isDependentMethodType)
-              DependentMethodTpeConversionToFunctionError(tree, tpe)
-            checkParamsConvertible(tree, restpe)
-          case _ =>
-        }
-      checkParamsConvertible0(tpe0)
-    }
-
     /** Check that type of given tree does not contain local or private
      *  components.
      */
@@ -803,10 +785,10 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
      *      unless followed by explicit type application.
      *  (4) Do the following to unapplied methods used as values:
      *  (4.1) If the method has only implicit parameters pass implicit arguments
-     *  (4.2) otherwise, if `pt` is a function type and method is not a constructor,
-     *        convert to function by eta-expansion,
-     *  (4.3) otherwise, if the method is nullary with a result type compatible to `pt`
+     *  (4.2) otherwise, if the method is nullary with a result type compatible to `pt`
      *        and it is not a constructor, apply it to ()
+     *  (4.3) otherwise, if `pt` is a function type and method is not a constructor,
+     *        convert to function by eta-expansion,
      *  otherwise issue an error
      *  (5) Convert constructors in a pattern as follows:
      *  (5.1) If constructor refers to a case class factory, set tree's type to the unique
@@ -900,62 +882,42 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
       def instantiateToMethodType(mt: MethodType): Tree = {
         val meth = tree match {
           // a partial named application is a block (see comment in EtaExpansion)
+          //
+          // TODO: document why we need to peel off one layer to begin with, and, if we do, why we don't need to recurse??
+          // (and why we don't need to look at the stats to make sure it's the block created by eta-expansion)
+          // I don't see how we call etaExpand on a constructor..
+          //
+          // I guess we don't need to recurse because the outer block and a nested block will refer to the same method
+          // after eta-expansion (if that's what generated these blocks), so we can just look at the outer one.
+          //
+          // How about user-written blocks? Can they ever have a MethodType?
           case Block(_, tree1) => tree1.symbol
           case _               => tree.symbol
         }
 
         def cantAdapt =
-          if (context.implicitsEnabled) MissingArgsForMethodTpeError(tree, meth)
-          else UnstableTreeError(tree)
-
-        def emptyApplication: Tree = adapt(typed(Apply(tree, Nil) setPos tree.pos), mode, pt, original)
+          if (context.implicitsEnabled) MissingArgsForMethodTpeError(tree, meth) else UnstableTreeError(tree)
 
         // constructors do not eta-expand
         if (meth.isConstructor) cantAdapt
-        // (4.2) eta-expand method value when function or sam type is expected
-        else if (isFunctionType(pt) || (!mt.params.isEmpty && samOf(pt).exists)) {
-          // scala/bug#9536 `!mt.params.isEmpty &&`: for backwards compatibility with 2.11,
-          // we don't adapt a zero-arg method value to a SAM
-          // In 2.13, we won't do any eta-expansion for zero-arg methods, but we should deprecate first
-
-          debuglog(s"eta-expanding $tree: ${tree.tpe} to $pt")
-          checkParamsConvertible(tree, tree.tpe)
-
-          // method values (`m _`) are always eta-expanded (this syntax will disappear once we eta-expand regardless of expected type, at least for arity > 0)
-          // a "naked" method reference (`m`) may or not be eta expanded -- currently, this depends on the expected type and the arity (the conditions for this are in flux)
-          def isMethodValue = tree.getAndRemoveAttachment[MethodValueAttachment.type].isDefined
-          val nakedZeroAryMethod = mt.params.isEmpty && !isMethodValue
-
-          // scala/bug#7187 eta-expansion of zero-arg method value is deprecated
-          // 2.13 will switch order of (4.3) and (4.2), always inserting () before attempting eta expansion
-          // (This effectively disables implicit eta-expansion of 0-ary methods.)
-          // See mind-bending stuff like scala/bug#9178
-          if (nakedZeroAryMethod && settings.isScala213) emptyApplication
-          else {
-            // eventually, we will deprecate insertion of `()` (except for java-defined methods) -- this is already the case in dotty
-            // Once that's done, we can more aggressively eta-expand method references, even if they are 0-arity
-            // 2.13 will already eta-expand non-zero-arity methods regardless of expected type (whereas 2.12 requires a function-equivalent type)
-            if (nakedZeroAryMethod && settings.isScala212) {
-              currentRun.reporting.deprecationWarning(tree.pos, NoSymbol,
-                                                       s"Eta-expansion of zero-argument methods is deprecated. To avoid this warning, write ${Function(Nil, Apply(tree, Nil))}.", "2.12.0")
-            }
-
-            val tree0 = etaExpand(context.unit, tree, this)
+        // (4.2) apply to empty argument list
+        else if (mt.params.isEmpty && (settings.isScala213 || !isFunctionType(pt))) {
+          // Starting with 2.13, always insert `()`, regardless of expected type.
+          // On older versions, expected type must not be a FunctionN (can be a SAM)
+          //
+          // scala/bug#7187 deprecates eta-expansion of zero-arg method values (since 2.12; was never introduced for SAM types: scala/bug#9536).
+          // The next step is to also deprecate insertion of `()` (except for java-defined methods), as dotty already requires explicitly writing them.
+          // Once explicit application to () is required, we can more aggressively eta-expand method references, even if they are 0-arity
+          adapt(typed(Apply(tree, Nil) setPos tree.pos), mode, pt, original)
+        }
+        // (4.3) eta-expand method value when function or sam type is expected (for experimentation, always eta-expand under 2.14 source level)
+        else if (isFunctionType(pt) || samOf(pt).exists || settings.isScala214) { // TODO: decide on `settings.isScala214`
+          if (settings.isScala212 && mt.params.isEmpty) // implies isFunctionType(pt)
+             currentRun.reporting.deprecationWarning(tree.pos, NoSymbol, "Eta-expansion of zero-argument methods is deprecated. "+
+                    s"To avoid this warning, write ${Function(Nil, Apply(tree, Nil))}.", "2.12.0")
 
-            // #2624: need to infer type arguments for eta expansion of a polymorphic method
-            // context.undetparams contains clones of meth.typeParams (fresh ones were generated in etaExpand)
-            // need to run typer on tree0, since etaExpansion sets the tpe's of its subtrees to null
-            // can't type with the expected type, as we can't recreate the setup in (3) without calling typed
-            // (note that (3) does not call typed to do the polymorphic type instantiation --
-            //  it is called after the tree has been typed with a polymorphic expected result type)
-            if (hasUndets)
-              instantiate(typed(tree0, mode), mode, pt)
-            else
-              typed(tree0, mode, pt)
-          }
+          typedEtaExpansion(tree, mode, pt)
         }
-        // (4.3) apply to empty argument list
-        else if (mt.params.isEmpty) emptyApplication
         else cantAdapt
       }
 
@@ -1645,7 +1607,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
             map2(preSuperStats, preSuperVals)((ldef, gdef) => gdef.tpt setType ldef.symbol.tpe)
 
           if (superCall1 == cunit) EmptyTree
-          else cbody2 match {
+          else cbody2 match { // ???
             case Block(_, expr) => expr
             case tree => tree
           }
@@ -3003,7 +2965,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         // This tree is the result from one of:
         //   - manual eta-expansion with named arguments (x => f(x));
         //   - wildcard-style eta expansion (`m(_, _,)`);
-        //   - instantiateToMethodType adapting a tree of method type to a function type using etaExpand.
+        //   - (I don't think it can result from etaExpand, because we know the argument types there.)
         //
         // Note that method values are a separate thing (`m _`): they have the idiosyncratic shape
         // of `Typed(expr, Function(Nil, EmptyTree))`
@@ -3082,6 +3044,23 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
       }
     }
 
+    // #2624: need to infer type arguments for eta expansion of a polymorphic method
+    // context.undetparams contains clones of meth.typeParams (fresh ones were generated in etaExpand)
+    // need to run typer on tree0, since etaExpansion sets the tpe's of its subtrees to null
+    // can't type with the expected type, as we can't recreate the setup in (3) without calling typed
+    // (note that (3) does not call typed to do the polymorphic type instantiation --
+    //  it is called after the tree has been typed with a polymorphic expected result type)
+    def typedEtaExpansion(tree: Tree, mode: Mode, pt: Type): Tree = {
+      debuglog(s"eta-expanding $tree: ${tree.tpe} to $pt")
+
+      if (tree.tpe.isDependentMethodType) DependentMethodTpeConversionToFunctionError(tree, tree.tpe) // TODO: support this
+      else {
+        val expansion = etaExpand(context.unit, tree, this)
+        if (context.undetparams.isEmpty) typed(expansion, mode, pt)
+        else instantiate(typed(expansion, mode), mode, pt)
+      }
+    }
+
     def typedRefinement(templ: Template): Unit = {
       val stats = templ.body
       namer.enterSyms(stats)
@@ -4613,14 +4592,6 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           treeCopy.New(tree, tpt1).setType(tp)
       }
 
-      def functionTypeWildcard(arity: Int): Type =
-        functionType(List.fill(arity)(WildcardType), WildcardType)
-
-      def checkArity(tree: Tree)(tp: Type): tp.type = tp match {
-        case NoType => MaxFunctionArityError(tree); tp
-        case _ => tp
-      }
-
 
       /** Eta expand an expression like `m _`, where `m` denotes a method or a by-name argument
         *
@@ -4633,9 +4604,12 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         */
       def typedEta(methodValue: Tree): Tree = methodValue.tpe match {
         case tp@(MethodType(_, _) | PolyType(_, MethodType(_, _))) => // (1)
-          val formals = tp.params
-          if (isFunctionType(pt) || samMatchesFunctionBasedOnArity(samOf(pt), formals)) methodValue
-          else adapt(methodValue, mode, checkArity(methodValue)(functionTypeWildcard(formals.length)))
+          val etaPt =
+            if (pt ne WildcardType) pt
+            else functionType(tp.params.map(_ => WildcardType), WildcardType) orElse WildcardType // arity overflow --> NoType
+
+          // We know syntactically methodValue can't refer to a constructor because you can't write `this _` for that (right???)
+          typedEtaExpansion(methodValue, mode, etaPt)
 
         case TypeRef(_, ByNameParamClass, _) |  NullaryMethodType(_) => // (2)
           val pos = methodValue.pos
@@ -5420,14 +5394,19 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           // that typecheck must not trigger macro expansions, so we explicitly prohibit them
           // however we cannot do `context.withMacrosDisabled`
           // because `expr` might contain nested macro calls (see scala/bug#6673).
-          // Otherwise, eta-expand, passing the original tree, which is required in adapt
-          // for trees of the form `f() _`: if the method type takes implicits, the fallback
-          // strategy will use `f()`; else if not, original is used to distinguish an explicit
-          // method value from eta-expansion driven by an expected function type.
+          // Otherwise, (check for dead code, and) eta-expand.
           case MethodValue(expr) =>
-            typed1(suppressMacroExpansion(expr), mode, pt) match {
+            // Need to type in FUNmode so that we accept a method type (which also means we can't use our pt),
+            // this does mean no overloading is performed. The main reason to ignore pt and move to FUNmode is that
+            // the `m` in `m _` could involve an implicit conversion, which will go through adapt after converting,
+            // which will run afoul of the restriction that a method-typed tree is only allowed when a function type is expected.
+            // We peeled off the `_` marker for the typed1 call, so we don't know that the user has requested eta-expansion.
+            // See scala/bug#8299.
+            val funTyped = typed1(suppressMacroExpansion(expr), mode | FUNmode, WildcardType)
+            if (funTyped.tpe.isInstanceOf[OverloadedType]) inferExprAlternative(funTyped, pt)
+            funTyped match {
               case macroDef if treeInfo.isMacroApplication(macroDef) => MacroEtaError(macroDef)
-              case methodValue                                       => typedEta(checkDead(methodValue).updateAttachment(MethodValueAttachment))
+              case methodValue                                       => typedEta(checkDead(methodValue))
             }
           case Typed(expr, tpt) =>
             val tpt1  = typedType(tpt, mode)                           // type the ascribed type first

test/files/neg/t7187-2.13.check

@@ -0,0 +1,11 @@
+t7187-2.14.scala:6: error: type mismatch;
+ found   : Int
+ required: () => Any
+  val t1: () => Any  = m1   // error
+                       ^
+t7187-2.14.scala:7: error: type mismatch;
+ found   : Int
+ required: () => Any
+  val t2: () => Any  = m2   // error, no eta-expansion of zero-args methods
+                       ^
+two errors found

test/files/neg/t7187-2.13.flags

@@ -0,0 +1 @@
+-Xsource:2.14

test/files/neg/t7187-2.13.scala

@@ -0,0 +1,25 @@
+class EtaExpand214 {
+  def m1 = 1
+  def m2() = 1
+  def m3(x: Int) = x
+
+  val t1: () => Any  = m1   // error
+  val t2: () => Any  = m2   // error, no eta-expansion of zero-args methods
+  val t3: Int => Any = m3   // ok
+
+  val t4 = m1 // apply
+  val t5 = m2 // apply, ()-insertion
+  val t6 = m3 // eta-expansion in 2.14
+
+  val t4a: Int        = t4 // ok
+  val t5a: Int        = t5 // ok
+  val t6a: Int => Any = t6 // ok
+
+  val t7 = m1 _
+  val t8 = m2 _
+  val t9 = m3 _
+
+  val t7a: () => Any  = t7 // ok
+  val t8a: () => Any  = t8 // ok
+  val t9a: Int => Any = t9 // ok
+}

test/files/neg/t7187-2.14.check

@@ -32,4 +32,9 @@ t7187.scala:25: error: type mismatch;
  required: () => Any
   val t4b: () => Any = zap()   // ditto
                           ^
-8 errors found
+t7187.scala:30: error: missing argument list for method zup in class EtaExpandZeroArg
+Unapplied methods are only converted to functions when a function type is expected.
+You can make this conversion explicit by writing `zup _` or `zup(_)` instead of `zup`.
+  val t5a = zup // error in 2.13, eta-expansion in 2.14
+            ^
+9 errors found