Skip to content

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
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