Strange error when exporting overloaded stuff from the collection library

[bjornregnell]

Compiler version

$ scala-cli repl . -S 3.nightly
Welcome to Scala 3.2.0-RC1-bin-20220415-8037f3b-NIGHTLY-git-8037f3b (11.0.11, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

Minimized code and Output

scala> class B(val s: Set[Int]):
     |   export s.*
     | 
-- [E120] Naming Error: --------------------------------------------------------
2 |  export s.*
  |           ^
  |Double definition:
  |final def concat
  |  (suffix: scala.collection.IterableOnce): scala.collection.immutable.Set in class B at line 2 and
  |final def concat
  |  (that: scala.collection.IterableOnce): scala.collection.immutable.Set in class B at line 2
  |have the same type after erasure.
  |
  |Consider adding a @targetName annotation to one of the conflicting definitions
  |for disambiguation.
1 error found

However if the concat methods are excluded then it works:

scala> class C(val s: Set[Int]):
     |   export s.{concat => _, ++ => _, *}
     | 
// defined class C

Expectation

Exporting all the Set stuff should work even with the concat methods.

[odersky]

There are indeed two conflicting members in Set, so the export as is does not work. But the error message was not helping since it showed the member types after erasure. The new error message is:

2 |  export s.* // error
           ^
           Double definition:
           final def concat[B >: T](suffix: IterableOnce[B]): Set[B] in class B at line 2 and
           final def concat(that: IterableOnce[T]): Set[T] in class B at line 2
           have the same type after erasure.

           Consider adding a @targetName annotation to one of the conflicting definitions
           for disambiguation.

It turns out the first concat comes from IterableOps and the second from SetOps. They don't clash since they live in different classes (i am not sure, maybe they shouls clash, but it's a Scala 2 library, so we can't change it). But
exporting them both produces two forwarders with the same name and JVM signature.

[som-snytt]

I don't know why the brain has to be atPhase(erasure) to figure this stuff out, but I feel sure that if the competing signatures are supported, they should be exportable.

[bjornregnell]

Thanks @odersky for explaining!

i am not sure, maybe they should clash, but it's a Scala 2 library, so we can't change it

So I guess there are some prioritization going on of some kind that makes it compile even if things are mixed in or implicitly summoned. But I guess it is possible to change how export prioritizes things when seeing this kind of ambiguity and actually select one of them in a wise manner, if possible?

The new error message

The new error message indeed has more informative types but it still feels strange that an ambiguity error surfaces when exporting code that already passed the compiler... If it makes sense then I think the compiler semantics of export should help me select one thing. Or is this a can of worms that I haven't appreciated the depth of?

[bjornregnell]

Also the hint "Consider adding a @TargetNAME annotation to one of the conflicting definitions" is not very helpful here as I have no clue how to add that @TargetNAME to the existing concat methods in Set.

[dwijnand]

They're overloads because they differ in erased result types: SetOps in the case of the concat in SetOps, Object in the case of IterableOps, as those are the erased types of the C and CC class type parameters.

[bjornregnell]

I could do my own forwarders, but then why bother with @TargetNAME as the error msg says?

$ scala-cli repl . -S 3.nightly
Welcome to Scala 3.2.0-RC1-bin-20220416-53f5531-NIGHTLY-git-53f5531 (11.0.11, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> class C[T](val s: Set[T]):
     |   export s.{concat => _, ++ => _, *}
     |   final def concat(that: IterableOnce[T]): Set[T] = s.concat(that)
     |   final def ++(that: IterableOnce[T]): Set[T] = s.concat(that)
     | 
// defined class C
                       

But this is boilerplate that I was able to do thank's to the old error message, and I didn't do the B >: T dance 😄

[bishabosha]

I could do my own forwarders, but then why bother with @TargetNAME as the error msg says?

$ scala-cli repl . -S 3.nightly
Welcome to Scala 3.2.0-RC1-bin-20220416-53f5531-NIGHTLY-git-53f5531 (11.0.11, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> class C[T](val s: Set[T]):
     |   export s.{concat => _, ++ => _, *}
     |   final def concat(that: IterableOnce[T]): Set[T] = s.concat(that)
     |   final def ++(that: IterableOnce[T]): Set[T] = s.concat(that)
     | 
// defined class C
                       

But this is boilerplate that I was able to do thank's to the old error message, and I didn't do the B >: T dance 😄

It seems in this case that the implementation of the explicit forwarder itself was able to select an unambiguous overload - so perhaps we can do the same to only export one of them (if it does not force too many things)

[dwijnand]

I'm really on the fence on whether or not it's a good idea to silently not emit a forwarder for one of the overloads. On the one hand, there are going to be times when you don't care. On the other hand, there are going to be times where you assume everything is being exported...

[dwijnand]

No doubt the targetName thing should be suppressed in light of the fact that the definitions are from the export, btw.

[bishabosha]

I'm really on the fence on whether or not it's a good idea to silently not emit a forwarder for one of the overloads. On the one hand, there are going to be times when you don't care. On the other hand, there are going to be times where you assume everything is being exported...

on the other hand, there are already times when the wildcard silently ignores exporting a definition. (like an overload of a pre-existing inherited member)

[bjornregnell]

I'm fine with the compiler choosing one of the overloaded things for me. If there are dangers I would be OK with a warning saying that "X was chosen but there is also Y" and then it's up to me to create a forwarder (preferably based on a helpful hint on how to write such a forwarder that I can copy-paste from the warning message if that is what I want). But I definitely think the export should work without a compile error, if that makes sense.

[som-snytt]

There is another discussion about why isn't it allowed to export an extension method where the member is defined, since they have different erased or actual signatures. Different use case but similar flavor. (#14497 (comment))