Fix #16654 and partially fix #16583

[mbovel]

Fixes #16654 and part of #16583.

[sjrd]

This is not backward tasty compatible, unfortunately.

[mbovel]

Does it mean it should be merged after the next minor release only, or that we can't do that at all?

[sjrd]

We cannot do it at all.

[mbovel]

Too bad 🥲
We have the same problem for Tuple.map by the way: #15992 (comment).

[bishabosha]

how can we sensibly improve API's known to be broken? to we need to add an .api2 method to Tuple which returns an object with all the fixed APIs?

[sjrd]

The only true way is not to introduce broken APIs in the first place ... which means more testing when they are introduced. Of course that's too late for what's already published.

It might be possible to pull off something once @binaryAPI is introduced. As we could keep the broken version as @binaryAPI private[Tuple] and introduce the fixed version as public.

[bishabosha]

As we could keep the broken version as @binaryapi private[Tuple] and introduce the fixed version as public.

I thought when inlining that would fail the accessibility check, as it would already be resolved to the specific overload? - would rechecks ignore @binaryAPI?

[sjrd]

Accessibility from tasty is already relaxed to a degree.

[mbovel]

Suggestion from Martin: deprecate Tuple.toList and replace it with a new Tuple.toIterator that has a type parameter.

Not sure what to do for Tuple.map.

[mbovel]

Following the discussion above, I rolled back the change to the signature of Tuple.toList.

As a result, while all other tests and minimization of #16583 are fixed by the compareAtoms and ApproximatingTypeMap fixes, labels can still not compile:

inline def labels[Labels <: Tuple](using ev: Tuple.Union[Labels] <:< String): List[String] =
  val tmp = constValueTuple[Labels].toList
  ev.substituteCo( tmp )

-- [E007] Type Mismatch Error: tests/pos/16583.scala:11:19 ---------------------
11 |  ev.substituteCo( tmp )
   |                   ^^^
   | Found:    (tmp : List[Any])
   | Required: List[Tuple.Union[Labels]]
   |
   | where:    Labels is a type in method labels with bounds <: Tuple
   |
   |
   | Note: a match type could not be fully reduced:
   |
   |   trying to reduce  Tuple.Union[Labels]
   |   trying to reduce  scala.Tuple.Fold[Labels, Nothing, [x, y] =>> x | y]
   |   failed since selector Labels
   |   does not match  case EmptyTuple => Nothing
   |   and cannot be shown to be disjoint from it either.
   |   Therefore, reduction cannot advance to the remaining case
   |
   |     case h *: t => h | scala.Tuple.Fold[t, Nothing, [x, y] =>> x | y]
   |
   | longer explanation available when compiling with `-explain`

This makes sense. Not sure how we could fix this without changing the signature of Tuple.toList.

Writing the expected type of tmp explicitly:

val tmp: List[Tuple.Union[Labels]] = (constValueTuple[Labels]: Labels).toList

-- [E007] Type Mismatch Error: tests/pos/16583.scala:10:73 ---------------------
10 |  val tmp: List[Tuple.Union[Labels]] = (constValueTuple[Labels]: Labels).toList
   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |Found:    List[Tuple.Union[(?1 : Labels)]]
   |Required: List[Tuple.Union[Labels]]
   |
   |where:    ?1     is an unknown value of type Labels
   |          Labels is a type in method labels with bounds <: Tuple
   |
   |
   |Note: a match type could not be fully reduced:
   |
   |  trying to reduce  Tuple.Union[(?1 : Labels)]
   |  trying to reduce  scala.Tuple.Fold[(?1 : Labels), Nothing, [x, y] =>> x | y]
   |  failed since selector (?1 : Labels)
   |  does not match  case EmptyTuple => Nothing
   |  and cannot be shown to be disjoint from it either.
   |  Therefore, reduction cannot advance to the remaining case
   |
   |    case h *: t => h | scala.Tuple.Fold[t, Nothing, [x, y] =>> x | y]
   |
   | longer explanation available when compiling with `-explain`

[mbovel]

Note: I kept individual commits for now to ease reviews, but will rebase to 2 commits (compareAtoms and ApproximatingTypeMap fixes) before merging.

Addressed.

[sjrd]

I have dismissed my earlier review since the API does not change anymore. However, I am woefully unqualified to sign off on the changes that do remain in the PR, so I'll refrain from posting a new review.

[mbovel]

Failing community test:

-- [E057] Type Mismatch Error: /__w/dotty/dotty/community-build/community-projects/perspective/dotty/examples/src/main/scala/perspective/examples/testing.scala:144:29 
144 |val res = Cat("Garfield").foo("name")
    |                             ^
    |Type argument ("name" : String) | Nothing does not conform to lower bound Any
    |---------------------------------------------------------------------------
    |Inline stack trace
    |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
    |This location contains code that was inlined from hkdGeneric.scala:87
 87 |      constValueTuple[m.MirroredElemLabels].toList.toSet.asInstanceOf[Set[String]]
    |                                                   ^
     ---------------------------------------------------------------------------
    |
    | longer explanation available when compiling with `-explain`
one error found
(dottyPerspectiveExamples / Compile / compileIncremental) Compilation failed
Total time: 16 s, completed Jun 1, 2023 3:00:53 PM

Code: https://github.com/dotty-staging/perspective/blob/365383df6a7b0c2eeb81742f479aa56269a4b557/dotty/examples/src/main/scala/perspective/examples/testing.scala#L137-L144.

At first sight, that's another case wher a type application argument is a skolem of an abstract type, resulting in the application being approximated. This is similar to the labels example above.

[smarter]

There's too much going on in this PR I think, can we split the AllowLambdaWildcardApply change in its own PR so we can evaluate it separately?

[dwijnand]

Found a way to make it all work, changing less: #18043

[mbovel]

Let's try to get #18043 merged first, and then to address the AllowLambdaWildcardApply change in a separate PR as suggested.