Skip to content

Fix #4582: use untpd.New in maybeCall #4827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 24, 2018
Merged

Fix #4582: use untpd.New in maybeCall #4827

merged 1 commit into from
Jul 24, 2018

Conversation

Medowhill
Copy link
Contributor

No description provided.

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

@@ -0,0 +1,2 @@
trait T[A: Numeric]
class TX[A: Numeric] extends T[A]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't exercice the code you changed but I would add a test with an argument list. E.g.

trait T2[A: Numeric](x: Int)
class T2X[A: Numeric] extends T[A](1)

Copy link
Contributor Author

@Medowhill Medowhill Jul 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just added it.

@allanrenucci allanrenucci merged commit 13d02bc into scala:master Jul 24, 2018
@Medowhill Medowhill deleted the issue4582 branch July 24, 2018 09:32
@@ -1475,8 +1475,7 @@ class Typer extends Namer
*/
def maybeCall(ref: Tree, psym: Symbol, cinfo: Type): Tree = cinfo.stripPoly match {
case cinfo @ MethodType(Nil) if cinfo.resultType.isImplicitMethod =>
val icall = New(ref).select(nme.CONSTRUCTOR).appliedToNone
typedExpr(untpd.TypedSplice(icall))(superCtx)
typedExpr(untpd.New(ref, Nil))(superCtx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love when people fix bugs by removing code! ❤️

@Blaisorblade
Copy link
Contributor

@Medowhill Could we also test the interaction with #4798? That is, try adding variants of those tests with context bounds. Hopefully that already just works!

@Medowhill Medowhill restored the issue4582 branch July 24, 2018 20:50
@Medowhill
Copy link
Contributor Author

Medowhill commented Jul 24, 2018

I think this issue is orthogonal to #4798 because #4798 is related to type aliases for classes rather than traits. For example, the following code is already compiled successfully at the master branch.

trait A[X]
object O {
  type T[X] = A[X]
  class B[X] extends T[X]
}

class B[X] extedns T[X] is transformed into class B[X] extends Object with A[X].

However, adding a test can be meaningful. Do you think it would be better to add the following test?

trait A[X: Numeric]
object O {
  type T[X] = A[X]
  class B[X: Numeric] extends T[X]
}

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Jul 24, 2018

Yep, I'm talking about adding tests — I only wondered because this code calls untpd.New, which you're changing. To test the interaction I was thinking of something like the following (which fails on master and works on your branch).

class A[X: Numeric]
type T[X, Y] = A[X]
type Foo[X, Y] = T[X, Y]
class B[X: Numeric] extends Foo[X, Int]

@Medowhill
Copy link
Contributor Author

I found a new issue while writing tests.

trait T[X]
type Foo[X, Y] = T[X]
class D[X] extends Foo[X, Unit]
trait T[X: Numeric]
type Foo[X] = T[X]
class D[X: Numeric] extends Foo[X]

Two above examples were compiled.

trait T[X: Numeric]
type Foo[X, Y] = T[X]
class D[X: Numeric] extends Foo[X, Unit]

However, this one resulted the following crash:

Exception in thread "main" java.lang.AssertionError: assertion failed: missing parameters for trait T from ($outer: C, implicit evidence$2: scala.math.Numeric) extends Object() with T {
  private implicit val evidence$2: scala.math.Numeric
  private val $outer: C
  private <accessor> def $outer(): C = this.$outer
  final def C$D$$$outer(): C = D.this.$outer()
} should have been caught in typer
	at scala.Predef$.assert(Predef.scala:219)
	at dotty.tools.dotc.transform.Mixin.nextArgument$1(Mixin.scala:209)
	at dotty.tools.dotc.transform.Mixin.$anonfun$transformTemplate$12(Mixin.scala:235)
	at scala.collection.TraversableLike$WithFilter.$anonfun$map$2(TraversableLike.scala:739)
	at scala.collection.immutable.List.foreach(List.scala:389)
	at scala.collection.TraversableLike$WithFilter.map(TraversableLike.scala:738)
	at dotty.tools.dotc.transform.Mixin.traitInits$1(Mixin.scala:217)
	at dotty.tools.dotc.transform.Mixin.$anonfun$transformTemplate$17(Mixin.scala:263)
	at scala.collection.immutable.List.flatMap(List.scala:335)
	at dotty.tools.dotc.transform.Mixin.transformTemplate(Mixin.scala:262)
	at dotty.tools.dotc.transform.Mixin.transformTemplate(Mixin.scala:98)
	at dotty.tools.dotc.transform.MegaPhase.goTemplate(MegaPhase.scala:922)

@allanrenucci
Copy link
Contributor

I found a new issue while writing tests.

Please open an issue

@Medowhill
Copy link
Contributor Author

I've just opened it: #4837.

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