Skip to content

Conversation

@smarter
Copy link
Member

@smarter smarter commented Mar 27, 2015

This way, tpd#applyOverloaded can safely be used after typer. This issue
was encoutered while working on value classes, step 3 of SIP-15 contains
the following peephole optimization:

  new C(e) == new C(f) => e == f

Which requires us to do overloading resolution.

Review by @odersky .

This way, tpd#applyOverloaded can safely be used after typer. This issue
was encoutered while working on value classes, step 3 of SIP-15 contains
the following peephole optimization:
  new C(e) == new C(f) => e == f
Which requires us to do overloading resolution.
@smarter
Copy link
Member Author

smarter commented Mar 27, 2015

If you're wondering, the assert is triggered in inferImplicit which is called by inferView which is called by viewExists.

@odersky
Copy link
Contributor

odersky commented Mar 28, 2015

Yes, looks like we have to do it this way. LGTM.

DarkDimius added a commit that referenced this pull request Mar 28, 2015
Implicits#viewExists: return false after typer instead of AssertionError
@DarkDimius DarkDimius merged commit 0cc73c6 into scala:master Mar 28, 2015
@retronym
Copy link
Member

Where is the overload resolution? See scala/scala#3497 / SI-8129. We determined that the overloaded nature of == was a bug and have since fixed this in Scala:

% ~/scala/2.10/bin/scala
Welcome to Scala version 2.10.4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_25).
Type in expressions to have them evaluated.
Type :help for more information.

scala> :power
** Power User mode enabled - BEEP WHIR GYVE **
** :phase has been set to 'typer'.          **
** scala.tools.nsc._ has been imported      **
** global._, definitions._ also imported    **
** Try  :help, :vals, power.<tab>           **

scala> typeOf[AnyRef].member(newTermName("==").encodedName).alternatives
res2: List[$r.intp.global.Symbol] = List(method ==, method ==)

% ~/scala/2.11/bin/scala
Welcome to Scala version 2.11.2 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_25).
Type in expressions to have them evaluated.
Type :help for more information.

scala> :power
** Power User mode enabled - BEEP WHIR GYVE **
** :phase has been set to 'typer'.          **
** scala.tools.nsc._ has been imported      **
** global._, definitions._ also imported    **
** Try  :help, :vals, power.<tab>           **

scala> typeOf[AnyRef].member(newTermName("==").encodedName).alternatives
res1: List[$r.intp.global.Symbol] = List(method ==)

@smarter
Copy link
Member Author

smarter commented Mar 30, 2015

== is overloaded in primitive classes like Int, see smarter@a477cb7 and #411 (diff)

@retronym
Copy link
Member

Oh, that makes sense.

An alternative fix might have been to arrange things such that ctx.mode is Mode.ImplicitsEnabled is always false post typer.

I suppose you could also avoid fully blown overload resolution with something like:

scala> def main_==(sym: Symbol) = sym.info.member(nme.EQ).suchThat(_.info.params.head.info.typeSymbol == sym)
main_$eq$eq: (sym: $r.intp.global.Symbol)$r.intp.global.Symbol

scala> main_==(symbolOf[Int]).defString
res12: String = def ==(x: Int): Boolean

scala> main_==(symbolOf[Char]).defString
res13: String = def ==(x: Char): Boolean

I only suggest this because in Scala2 we have a dev warning when overload resolution happens post typer, as this has occasionally shown up as a symptom of a bug. It would be nice to have the same sort of clean line in the sand in dotty.

@DarkDimius
Copy link
Contributor

@retronym this commit actually disables implicit search after typer entirely.
Previously it was failing with assertion. Now it will just not infer anything.

@smarter
Copy link
Member Author

smarter commented Mar 30, 2015

@retronym : Maybe, but you still need to check if the underlying type is a value class or not, because you cannot do:

scala> main_==(symbolOf[String]).defString
res1: String = <none>

And I wanted to use applyOverloaded precisely because I didn't want to reimplement this logic by hand, but if you think it's not worth it I'm not opposed to changing it

@retronym
Copy link
Member

I don't feel too strongly either way, just wanted to make sure the different possibilities were considered.

Here's a fun test that seems to argue in favour of your approach:

scala> class Id(val a: Any) extends AnyVal
defined class Id

scala> class Evil { def ==(other: Evil) = false }
defined class Evil

scala> val e = new Evil
e: Evil = Evil@5025a98f

scala> e == e
res4: Boolean = false

scala> (e: Any) == e
res5: Boolean = true

scala> new Id(e) == new Id(e)
res6: Boolean = false

scala> new Id(e) == (new Id(e): Any)
res7: Boolean = true

AFAICT this is as specced, as SIP-15 specifies the synthetic equals method and the peephole optimizations syntactically. So I suppose full overload resolution is really required.

tgodzik added a commit to tgodzik/scala3 that referenced this pull request Jul 3, 2025
Backport "Add inlay hints for by-name parameters" to 3.3 LTS
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.

4 participants