Macro debugging help

[Paul Butcher]

Eugene,

I've had a bug reported against ScalaMock that I'm struggling to diagnose. I'm clearly doing something wrong with types in the tree that I'm generating, but I'm not sure what. I suspect that the problem is either in the way that I'm generating type arguments to methods:

https://github.com/paulbutcher/ScalaMock/blob/master/core/src/main/scala/org/scalamock/Mock.scala#L209

Or in the way that I'm generating the parameter types themselves:

https://github.com/paulbutcher/ScalaMock/blob/master/core/src/main/scala/org/scalamock/Mock.scala#L181

I'd be very grateful for any pointers you might give me for where I might start looking, or if anything I'm doing strikes you as clearly wrong.

(This is all with 2.10.0, so I can't make use of quasiquotes or any of the other nice things in macro-paradise).

Thanks!

Here's the bug:

https://github.com/paulbutcher/ScalaMock/issues/24

Here's a small Java class that invokes the bug:

https://github.com/paulbutcher/ScalaMock/blob/master/core_tests/src/main/java/com/paulbutcher/test/JavaInterface.java

And here's a console log that illustrates the problem, with the tree I'm generating (both raw and prettyprinted):

Welcome to Scala version 2.10.0 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_09).

Type in expressions to have them evaluated.

Type :help for more information.

scala> object M extends org.scalamock.Mock

defined module M

scala> M.mock[com.paulbutcher.test.JavaInterface2](null)

------------

TypeApply(Select(Block(List(ClassDef(Modifiers(FINAL), newTypeName("$anon"), List(), Template(List(TypeTree()), emptyValDef, List(DefDef(Modifiers(), nme.CONSTRUCTOR, List(), List(List()), TypeTree(), Block(List(Apply(Select(Super(This(tpnme.EMPTY), tpnme.EMPTY), nme.CONSTRUCTOR), List())), Literal(Constant(())))), DefDef(Modifiers(OVERRIDE), newTermName("generic"), List(TypeDef(Modifiers(DEFERRED | PARAM), newTypeName("T"), List(), TypeBoundsTree(TypeTree(), TypeTree()))), List(List(ValDef(Modifiers(PARAM), newTermName("x$1"), TypeTree(), EmptyTree))), TypeTree(), Apply(Select(Select(This(newTypeName("$anon")), newTermName("mock$generic$0")), newTermName("apply")), List(Ident(newTermName("x$1"))))), ValDef(Modifiers(), newTermName("mock$generic$0"), TypeTree(), Apply(Select(New(AppliedTypeTree(Ident(org.scalamock.MockFunction1), List(TypeTree(), TypeTree()))), nme.CONSTRUCTOR), List(Literal(Constant(null)), Apply(Select(Select(Ident(newTermName("scala")), newTermName("Symbol")), newTermName("apply")), List(Literal(Constant("generic"))))))))))), Apply(Select(New(Ident(newTypeName("$anon"))), nme.CONSTRUCTOR), List())), newTermName("asInstanceOf")), List(TypeTree()))

------------

{

  final class $anon extends com.paulbutcher.test.JavaInterface2 {

    def <init>() = {

      super.<init>();

      ()

    };

    override def generic[T >: Nothing <: Any](x$1: T): Unit = $anon.this.mock$generic$0.apply(x$1);

    val mock$generic$0 = new MockFunction1[T, Unit](null, scala.Symbol.apply("generic"))

  };

  new $anon()

}.asInstanceOf[com.paulbutcher.test.JavaInterface2]

------------

<console>:9: error: overloaded method value apply with alternatives:

  (v1: T(in value mock$generic$0))Unit <and>

  (v1: (some other)T(in value mock$generic$0))Unit

 cannot be applied to (T)

              M.mock[com.paulbutcher.test.JavaInterface2](null)

                                                         ^

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

[Eugene Burmako]

In this situation dumping with printIds = true and printKinds = true might reveal differences between a mock for Scala generic and a mock for Java generic. Other than that I have no immediate idea. Let me take a look.

[Eugene Burmako]

This looks relevant: https://groups.google.com/forum/#!topic/scala-internals/YQOgtdNKNn0

On Jan 11, 8:29 pm, Eugene Burmako <eugene.burm...@epfl.ch> wrote:
> In this situation dumping with printIds = true and printKinds = true might
> reveal differences between a mock for Scala generic and a mock for Java
> generic. Other than that I have no immediate idea. Let me take a look.
>

> On 11 January 2013 13:48, Paul Butcher <p...@paulbutcher.com> wrote:
>
>
>
>
>
>
>
> > Eugene,
>
> > I've had a bug reported against ScalaMock that I'm struggling to diagnose.
> > I'm clearly doing something wrong with types in the tree that I'm
> > generating, but I'm not sure what. I suspect that the problem is either in
> > the way that I'm generating type arguments to methods:
>

> >https://github.com/paulbutcher/ScalaMock/blob/master/core/src/main/sc...

>
> > Or in the way that I'm generating the parameter types themselves:
>

> >https://github.com/paulbutcher/ScalaMock/blob/master/core/src/main/sc...

>
> > I'd be very grateful for any pointers you might give me for where I might
> > start looking, or if anything I'm doing strikes you as clearly wrong.
>
> > (This is all with 2.10.0, so I can't make use of quasiquotes or any of the
> > other nice things in macro-paradise).
>
> > Thanks!
>
> > Here's the bug:
>
> >https://github.com/paulbutcher/ScalaMock/issues/24
>
> > Here's a small Java class that invokes the bug:
>

> >https://github.com/paulbutcher/ScalaMock/blob/master/core_tests/src/m...

> > MSN: p...@paulbutcher.com
> > AIM: paulrabutcher
> > Skype: paulrabutcher

[Eugene Burmako]

Scala mock: https://gist.github.com/4513543

Java mock: https://gist.github.com/4513537

Note the prettyprinted type org#7.scalamock#6969.MockFunction0#8943[_] of mock$foo$0#20707 in the Java case. I guess typer does some transformations for symbols which originate from Java world (we even have a special flag for those), hence the mismatch.

But what really caught my eye is "m.typeParams map TypeDef _" in https://github.com/paulbutcher/ScalaMock/blob/master/core/src/main/scala/org/scalamock/Mock.scala#L209. Unfortunately, you can't do this, and I'm surprised it didn't blow up earlier. Sharing symbols between def trees is like playing with fire.

What I suggest is to either: 1) define mock$foo inside foo, so that it can legitimately see its type parameter, or 2) turn mock$foo into a polymorphic method and explicitly specify its type parameter. Probably not going to be easy :(

[Eugene Burmako]

Oh wait, it looks like you're using symbols of the originally mocked method to create an override and an underlying mock$blah:

  val forwarders = methodsToMock map forwarderImpl _

Unfortunately you can't do that for the same reason as described above. /cc Jason

[Paul Butcher]

Dang. I had a horrible feeling you were going to say something like that.

I guess that I'm going to have to do something that picks the types apart and then builds them up again "from scratch"?

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

[Eugene Burmako]

You mean for this part? https://github.com/paulbutcher/ScalaMock/blob/master/core/src/main/scala/org/scalamock/Mock.scala#L181

Probably you'll get away with substituteSymbols.

[Paul Butcher]

On 12 Jan 2013, at 08:04, Eugene Burmako <eugene....@epfl.ch> wrote:

You mean for this part? https://github.com/paulbutcher/ScalaMock/blob/master/core/src/main/scala/org/scalamock/Mock.scala#L181

Probably you'll get away with substituteSymbols.

Yes. I've not come across substitueSymbols before - is there anything I can get a look at to understand how it might help here?

Thanks (as always).

[Eugene Burmako]

From what I understand, one of the problems you'll need to solve is replacing usages of old symbols originating from the mockee meth with fresh symbols created exclusively for mocks. substituteSymbols is an easy solutions. Don't know any good examples, but its usage should be pretty straightforward.

[Paul Butcher]

On 12 Jan 2013, at 16:31, Eugene Burmako <eugene....@epfl.ch> wrote:

From what I understand, one of the problems you'll need to solve is replacing usages of old symbols originating from the mockee meth with fresh symbols created exclusively for mocks. substituteSymbols is an easy solutions. Don't know any good examples, but its usage should be pretty straightforward.

I've been trying to work out how to use substituteSymbols without any luck. I wonder if you could point me in the right direction?

I've created a new project, which is a cut-down version of the code in ScalaMock3 (to get some of the "noise" out of the way):

https://github.com/paulbutcher/implementor

This implements a macro called "implement" that, given a trait, returns an instance of that trait where each member is defined to return null. For example:

scala> import org.scalamock.Implement._

import org.scalamock.Implement._

scala> trait Foo { def m(x: Int, y: Double): String }

defined trait Foo

scala> val f = implement[Foo]

------------

TypeApply(Select(Block(List(ClassDef(Modifiers(FINAL), newTypeName("$anon"), List(), Template(List(TypeTree()), emptyValDef, List(DefDef(Modifiers(), nme.CONSTRUCTOR, List(), List(List()), TypeTree(), Block(List(Apply(Select(Super(This(tpnme.EMPTY), tpnme.EMPTY), nme.CONSTRUCTOR), List())), Literal(Constant(())))), DefDef(Modifiers(OVERRIDE), newTermName("m"), List(), List(List(ValDef(Modifiers(PARAM), newTermName("x"), TypeTree(), EmptyTree), ValDef(Modifiers(PARAM), newTermName("y"), TypeTree(), EmptyTree))), TypeTree(), TypeApply(Select(Literal(Constant(null)), newTermName("asInstanceOf")), List(TypeTree()))))))), Apply(Select(New(Ident(newTypeName("$anon"))), nme.CONSTRUCTOR), List())), newTermName("asInstanceOf")), List(TypeTree()))

------------

{

  final class $anon extends Foo {

    def <init>() = {

      super.<init>();

      ()

    };

    override def m(x: Int, y: Double): String = null.asInstanceOf[String]

  };

  new $anon()

}.asInstanceOf[Foo]

------------

f: Foo = $anon$1@341db69c

scala> f.m(1, 2)

res1: String = null

I've written the code to define new type parameters for type-parameterized methods:

https://github.com/paulbutcher/implementor/commit/245529c2953d943dc365c26ad343ebb6c1ea0f57

And this seems to work fine. But I'm struggling to work out how to rewrite this:

https://github.com/paulbutcher/implementor/blob/master/core/src/main/scala/org/scalamock/Implement.scala#L31

to use substituteSymbols.

Working out which symbols to substitute is easy enough - it's just the typeParams of the method that's being implemented. But where do I get the symbols to substitute them with? I've created a set of TypeDefs for the new type parameters, but there's no way (that I'm aware of) to get a symbol from a TypeDef?

I'm sure that I'm missing something simple - can you point me at it? :-)

Thanks!

[Jason Zaugg]

On Sun, Jan 13, 2013 at 3:56 PM, Paul Butcher <pa...@paulbutcher.com> wrote:

I've written the code to define new type parameters for type-parameterized methods:

https://github.com/paulbutcher/implementor/commit/245529c2953d943dc365c26ad343ebb6c1ea0f57

And this seems to work fine. But I'm struggling to work out how to rewrite this:

https://github.com/paulbutcher/implementor/blob/master/core/src/main/scala/org/scalamock/Implement.scala#L31

to use substituteSymbols.

Working out which symbols to substitute is easy enough - it's just the typeParams of the method that's being implemented. But where do I get the symbols to substitute them with? I've created a set of TypeDefs for the new type parameters, but there's no way (that I'm aware of) to get a symbol from a TypeDef?

I'm sure that I'm missing something simple - can you point me at it? :-)

You've distilled the essence of the problem, and it's a thorny one. Eugene and I were discussing variations of this last week.

Before we discuss a path forward, I'll point out some more flavours of this (some are more general than your use case).

trait X { 

  type XX

}

class T[A <: X, B <: A](val b: B) {

  type C = B

  def meth[D <: C with AnyRef](d: D)(dd: d.type)(ba: b.XX)(th: T.this.type): d.type

}

//

// object T$1 extends ImplementTypeMacro[T[A, A]](new A{})

//

object T$1 extends T[X, X](new A{}) {

   type C = A

   def meth[D$1 <: A](d$1: D$1)(dd$1: d$1.type)(ba$1: b.XX)(th$1: T$1.type): d$1.type = ???

}

This gives you an idea of how much substitution might be needed.

But even if you're willing to take on the challenge, you can't really get started because the new symbols for the type and value parameters that you generate aren't assigned and entered into the symbol table until after your macro has executed.

How do we handle this in the compiler itself? With painstaking use of direct symbol creation and substitution. It's an error prone task, even for experts. A good example is the implementation of Value Classes [1]. You need only browse through that code, and its history, to see how tough it can be. And it is still buggy! Copying signatures is hard enough, copying method bodies leads to extremely subtle bugs like SI-6891 [2]

Macro authors don't have access to these low-level symbol-twiddling facilities (without casting to reflect.internal._). Instead we ask you to create the entire tree, which we will subsequently assign symbols / type check.

Perhaps some sort of lazy trees could get us out of this chicken/egg problem, ie rather than creating a DefDef, you create a:

   LazyDefDef((owner: Tree) => { val tParams = mkTParams(owner); LazyDefDefTParams(tParams, (tparams: List[Symbol]) => ....))

Here, the macro code would yield one scope at a time (tparams, params1, ..., paramsN, return type, body). At each step, you'd have access to the symbols for the enclosing scope you created at the previous step. Consider this a left-field idea for now, I've no real idea about how this would pan out.

Long story short, I'm not sure you can implement your macro through the public macro API, and even if you used the internal API and started assigning symbols yourself, it will be challenging. But perhaps someone else will see something I'm missing.

I wish we had better answers for these problems. If we did, it would benefit compiler developers as much as macro authors.

-jason

[1] https://raw.github.com/scala/scala/master/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala

[2] https://github.com/retronym/scala/commit/ae28b2

[Paul Butcher]

On 13 Jan 2013, at 16:15, Jason Zaugg <jza...@gmail.com> wrote:

You've distilled the essence of the problem, and it's a thorny one.

Ah. Well at least that gives me confidence that I'm not being entirely stupid :-D

Long story short, I'm not sure you can implement your macro through the public macro API, and even if you used the internal API and started assigning symbols yourself, it will be challenging. But perhaps someone else will see something I'm missing.

Dang.

I'm not sure that I'm quite ready to give up just yet. It would be extremely galling to do so - I've already given up on the compiler plugin-based ScalaMock and I really thought that macros were the solution to those problems. And it's so very close to working - I have a huge pile of passing tests :-)

Happily, I don't need to tackle the whole problem - I'm only interested in parameter types and don't need to worry about method bodies at all. And I'm also only interested in types that can be instantiated (i.e. no need to worry about List[_], only List[Int]).

The non-existent symbol problem is only a problem if I want to use substituteSymbols. But I don't have to go down that road. I can "just" (for large values of "just") construct the tree that represents what the user would type if they were doing it manually. I'm not sure how large a problem that is, but I'm hoping it's tractable.

Please feel free to disabuse me before I dig myself into a hole I can't dig myself out of though :-)

[Paul Butcher]

On 13 Jan 2013, at 16:15, Jason Zaugg <jza...@gmail.com> wrote:

You've distilled the essence of the problem, and it's a thorny one.

Oh - one further thought. Is there any way to have the macro runtime detect when a macro writer makes this kind of mistake? There was a discussion about something like this on scala-internals a while ago:

https://groups.google.com/d/topic/scala-internals/SSVg2UGzFcw/discussion

The problem with the current status quo is that it's very easy to create something that:

a) looks reasonable to the untrained eye

b) typechecks and compiles

c) apparently works just fine, until you give it something "complicated"

This is recipe for heartbreak on the part of a macro writer. Especially as things currently stand with limited documentation and examples. I've had to work things out by trial and error, and in the absence of a true understanding of "the right way" it's easy to assume that if something compiles and (apparently) works, it's right :-)

[Paul Butcher]

So I've made a start on knocking something together, and (so far at least) it looks like it might have legs. There's certainly a way to go before I cover all the cases, but I'm hopeful that it'll prove tractable:

https://github.com/paulbutcher/implementor/commit/f1921de2b7de3d5ea8cf7f230c8e4e9f8c7f4b26

If there are lurking dragons that I should be aware of, I'd be very grateful if you could point them out.

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

[Jason Zaugg]

On Mon, Jan 14, 2013 at 2:18 AM, Paul Butcher <pa...@paulbutcher.com> wrote:

So I've made a start on knocking something together, and (so far at least) it looks like it might have legs. There's certainly a way to go before I cover all the cases, but I'm hopeful that it'll prove tractable:

https://github.com/paulbutcher/implementor/commit/f1921de2b7de3d5ea8cf7f230c8e4e9f8c7f4b26

If there are lurking dragons that I should be aware of, I'd be very grateful if you could point them out.

It looks like that path would lead to reimplemention of reifyType (Type => Tree) and symbol substitution. Dragons are probably some of the friendlier creatures you'll encounter :)

But persisting for a bit longer is probably worthwhile as an educational exercise.

-jason

[Paul Butcher]

On 14 Jan 2013, at 10:47, Jason Zaugg <jza...@gmail.com> wrote:

But persisting for a bit longer is probably worthwhile as an educational exercise.

Thanks Jason. Is that a kind way of saying "keep going down this road just long enough to realise that you're doing something stupid"? :-)

[Jason Zaugg]

On Sunday, January 13, 2013 5:54:38 PM UTC+1, Paul Butcher wrote:

Oh - one further thought. Is there any way to have the macro runtime detect when a macro writer makes this kind of mistake? There was a discussion about something like this on scala-internals a while ago:

https://groups.google.com/d/topic/scala-internals/SSVg2UGzFcw/discussion

The problem with the current status quo is that it's very easy to create something that:

a) looks reasonable to the untrained eye

b) typechecks and compiles

c) apparently works just fine, until you give it something "complicated"

This is recipe for heartbreak on the part of a macro writer. Especially as things currently stand with limited documentation and examples. I've had to work things out by trial and error, and in the absence of a true understanding of "the right way" it's easy to assume that if something compiles and (apparently) works, it's right :-)

In the course of fixing a bug with Value Classes, I took the first steps to adding some checks about tree invariants. This will be incuded in 2.10.1

By adding `-Ycheck:typer` to your compiler options, as I did in this commit [1], you are informed that you've messed up the symbol structure. You might also find `-uniqid -Xprint:typer` helpful, that will print the symbol ID in addition to the symbol names, and show the entire tree at the end of the typer phase.

-jason

[check: typer] The symbol, tpe or info of tree `(override def m[NEW_A >: Nothing <: Any](x: A, y: Double): String = null.asInstanceOf[String]) : [NEW_A](x: A, y: Double)String` refers to a out-of-scope symbol, type A. tree.symbol.ownerChain: method m, anonymous class $anon, value <local Test>, object Test, package test, package scalamock, package org, package <root>

[check: typer] The symbol, tpe or info of tree `(val x: A = _) : A` refers to a out-of-scope symbol, type A. tree.symbol.ownerChain: value x, method m, anonymous class $anon, value <local Test>, object Test, package test, package scalamock, package org, package <root>

[warn] TreeCheckers detected non-compliant trees in Aggregatable.scala

[warn] one warning found 

[1] https://github.com/retronym/implementor/commit/ebbba7d708a8fd576a2dc37dc825162a29efff30#commitcomment-2591805