Fix:Resolve ambiguous overload between empty param list and v…

[krrish175-byte]

Closing issue

closes #24871

[som-snytt]

Just for fun, alternative syntax for the boolean expression:

       tp1 match
         case tp1: MethodType => // (1)
-          tp1.paramInfos.isEmpty && tp2.isInstanceOf[LambdaType]
-          || {
+          tp1.paramInfos.isEmpty && tp2.isInstanceOf[LambdaType] && tp2.match
+            case tp2: MethodType => !tp2.isVarArgsMethod
+            case _ => true
+          || locally:
             if tp1.isVarArgsMethod then
               tp2.isVarArgsMethod
               && isApplicableMethodRef(alt2, tp1.paramInfos.map(_.repeatedToSingle), WildcardType, ArgMatch.Compatible)
             else
               isApplicableMethodRef(alt2, tp1.paramInfos, WildcardType, ArgMatch.Compatible)
-          }

experimenting with "selection of match" and also "locally is pronounced parens". I recently saw locally used this way.

The test includes a top-level expression, but really the expectation is that other tests should fail.

The question to ask here is whether the fix still obeys item (1).

Worth adding that github allows opening a "draft" PR, to work out the kinks.

[odersky]

While the reasoning behind the PR sound, the code can be simplified.

[odersky]

isVarArgsMethod works also for non-method types, so the whole logic can be simplified to

tp1.paramInfos.isEmpty 
&& tp2.isInstanceOf[LambdaType]
&& !tp2.isVarArgsMethod

[odersky]

Thanks for the quick fix!

[som-snytt]

The test still doesn't compile, and I'm afraid the regression test is already in at #24876.

I wasn't convinced on the ticket that the fix is desirable, and I'm curious to see if it also changes the "ambiguous extension" test.

[krrish175-byte]

@som-snytt
Thanks for the feedback! I'll investigate the compilation issue with the test. Could you point me to which specific test isn't compiling?

Regarding #24876 , I see your concern about whether this fix is desirable. The core issue I was addressing is that when choosing between an empty parameter list () and a varargs method, the empty list should be preferred as more specific. However, I understand this might have broader implications.

I'll check how this change affects the "ambiguous extension" test you mentioned. Could you point me to that test file so I can verify the behavior?

[som-snytt]

The test file is an ordinary compilation unit, so a top-level expression isn't supported.

[som-snytt]

@krrish175-byte At the bottom of the page, "Some checks..." lists jobs, with a red x at the failed one. Clicking that shows results, with the failure somewhere.

[krrish175-byte]

@som-snytt
Thanks for the feedback! I've wrapped the for-yield expression inside a test method within an object Test to make it a valid compilation unit. The fix has been pushed.

[krrish175-byte]

Hi, any update on this pr?

[som-snytt]

As you can see from "checks were not successful", the test still fails. Also, the regression test tests/neg/i24871.scala would fail and need to be deleted.

You must run the test under sbt with testCompilation i24871, see testing at
https://dotty.epfl.ch/docs/contributing/index.html

Compilation failed for: 'tests/pos/i24871.scala'                                
-- [E051] Reference Error: tests/pos/i24871.scala:12:17 ----------------------------------------------------------------
12 |      List(x) <- X()(.0)
   |                 ^
   |                 Ambiguous overload. The overloaded alternatives of method apply in object X with types
   |                  ()(d: scala.Double): List[List[scala.Double]]
   |                  (xs: scala.Int*)(d: scala.Double): scala.collection.immutable.Nil.type
   |                 both match arguments ()((0.0d : scala.Double))
   |
   | longer explanation available when compiling with `-explain`

Where tp1 is nullary, the added condition turns the first expression from true to false, so that the second expression must be evaluated, where the isApplicable is always true. The change doesn't change the result, but requires additional work.

The code comment is not correct. tp1 is not as good if tp2 is not applicable (it is more discriminating). (It's late here. I can check the words again in the morning.)

The ticket is about several overloads in several param lists, which remains ambiguous. (I think that is correct or reasonable.)