-
Notifications
You must be signed in to change notification settings - Fork 16
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
Migrating from untyped macros #2
Comments
Hi, It would certainly be great to get rid of untyped macros, but I can't think of how to do it right now. To make your solution work, we would need to have implicit view for every instance of monad/functor separately, e.g. I'll keep trying to come up with something and, of course, all the ideas are highly appreciated. |
Well, providing the views for every functor is not that high a price to pay :) As for the messing, I believe
That’s true. Still can be done with an “explicit” implicit though, which is of course ugly.
This is an interesting matter. How do you know anyway—with the current implementation—that it’s not |
By the way, I just realized that the new macro annotations expand before type-checking! So conceptually a ported version could look like so: @workflow[Option[_]] val x = Some(3) + 5 However, I just tried to pass the type argument through and it didn’t work quite well. It’s also unclear how to pass normal arguments, if we want |
That's how rewriting works — we start with the most nested subexpression. Basically, the whole expression never gets to be typechecked as is (unless we walked through all of its subexpressions and left them untouched). In this case, |
What exactly didn't work? |
class Workflow[A] extends StaticAnnotation {
def macroTransform(annottees: Any*) = macro WorkflowMacros.macroTransform[A]
}
object WorkflowMacros {
def macroTransform[A: c.WeakTypeTag](c: MacroContext)(annottees: c.Expr[Any]*): c.Expr[Any] = {
import c.universe._
println(weakTypeOf[A])
c.Expr[Any](Block(DefDef(Modifiers(), newTermName("x"), List(), List(), TypeTree(), Apply(Select(Literal(Constant(3)), newTermName("$plus")), List(Literal(Constant("asd"))))), Literal(Constant(()))))
}
}
So the annotation is not doing anything at all. Without the type arg it works fine: class Workflow extends StaticAnnotation {
def macroTransform(annottees: Any*) = macro WorkflowMacros.macroTransform
}
object WorkflowMacros {
def macroTransform(c: MacroContext)(annottees: c.Expr[Any]*): c.Expr[Any] = {
import c.universe._
//println(weakTypeOf[A])
c.Expr[Any](Block(DefDef(Modifiers(), newTermName("x"), List(), List(), TypeTree(), Apply(Select(Literal(Constant(3)), newTermName("$plus")), List(Literal(Constant("asd"))))), Literal(Constant(()))))
}
}
Another question is if we have a macro annotation with arguments ( |
Another issue is that we need higher-kinded type as the argument. I seem to remember, that |
I understand that, but what happens in this example? workflow[Option] {
Some(Success(42)) getOrElse "foobar"
} is it |
The first one. Method invocation is represented somewhat like Whereas workflow[Option] {
Some(Failure(new Exception)) getOrElse "foobar"
} will produce |
a) That's a bug: scalamacros/paradise#2. Probably would be easy to fix, but I don't have much time for that. If you need this fix badly, I can carry it out tomorrow. Otherwise, let's keep it on-demand.
|
@aztek It makes sense! Thanks for the explanation. @xeno-by re 2) Lol now I remember that you suggested the same when we submitted https://issues.scala-lang.org/browse/SI-7736 :) So I’ll also try to get the type args with it and see how it goes. Upd: even if I just put |
Check this out! scala> @workflow(option) val x = Some("f") + Some("o")*2 + Some("bar")
x: Option[String] = Some(foobar) However: scala> @workflow(option) val x = Some(2) + 3
error: ambiguous reference to overloaded definition,
both method + in class Int of type (x: Char)Int
and method + in class Int of type (x: Short)Int
match expected type ?
<console>:10: error: type mismatch;
found : Int(3)
required: String
@workflow(option) val x = Some(2) + 3
^ I am curious if this is an issue specific to how the tree is parsed/checked, e.g. https://github.com/stanch/scala-workflow/blob/506fcc75196406ce59ea5781630ea673768f8b50/core/src/main/scala/scala/workflow/package.scala#L195 |
Oh, cool! I'm not sure about the error. Looks like the whole expression is typechecked at some point (you'll get the same error message without the annotation). Generated code is correct, so rewriter/typechecker worked well. |
I think the first error happens somewhere in @context(option)
def foo(bar: Int) = {
@$ val a = 4 // a = Some(4)
a
} Of course the annotations are not as good-looking as functions, and you can put them into fewer places. But on the other hand, I believe we can do class async extends workflow(future)
@async val x = 5 // x = Future(5) which is not bad! Upd: The shown implementation of |
@xeno-by, after some investigation this looks very suspicious. println(util.Try(c.typeCheck(q"4+")))
...
error: ambiguous reference to overloaded definition,
both method + in class Int of type (x: Char)Int
and method + in class Int of type (x: Short)Int
match expected type ?
Success(4.<$plus: error>) The |
Most likely a bug. Would be great if you could submit it. As a workaround, On Sunday, 1 September 2013, stanch [email protected] wrote:
|
Is that it https://issues.scala-lang.org/browse/SI-7461 ? |
@xeno-by should I submit a new one, or we reopen this one? |
Dunno yet. Let me take a look first |
@stanch The example from SI-7461 works for me, so maybe there's a problem with the particular tree that you're emitting. Could you provide a minimized reproduction and reopen the bug with it? |
I know you are both watching that bug, but just in case—the reproduction is here https://github.com/stanch/paradise-bug. And now to something completely different. I realized this morning that by macro-annotating the enclosing method and defining |
@stanch Could ypu please put up some examples of macro-annotated workflows? Or, even better, revise some of the examples in the current specs. I don't think anybody actually uses I don't want to abandon the current implementation completely, so I'd rather have two branches here — one with release version and another with old untyped macros. If you get anything useful with annotations, it's fine with me to merge it here. There're parts of the code independent to macro interface (like instances and composition classes, that I'll push soon), that should be easy to sync. @xeno-by Anyway, do you think macro annotations will make their way to release? It's just it looks like we would switch untyped macros implementation with another untyped-at-heart implementation. Is it possible, that macro annotations will be eventually discarded for the same reasons untyped macros were? |
I will put the examples in I’ll be happy to submit a pull request with something useful, probably having a separate branch is indeed the best solution. [1] https://issues.scala-lang.org/browse/SI-7461 |
1 can be worked around, can't it? |
@xeno-by Maybe it can. To maintain the code here [1], I need a way to get the error description from the erroneous tree. I did a quick search through auto-completion with no success... |
Well, that's impossible from what I know. I'll try to see what can be done this weekend. |
@stanch To clear things up about the
|
@aztek Thanks! I kinda understood this, but your explanation makes it more clear. The current API is:
It seems that for the future API we have the following options:
Personally I think keeping everything would be a mess. So I suggest to select the most reasonable subset of the following (usage-wise):
I am leaning towards Please let me know what you guys think. |
@xeno-by I just found a rather upsetting issue: val x = Some(3)
@workflow(option) val y = "asd" + x fails during annotation expansion with |
Sorry for the confusion, by "you" I meant the programmer. My initial question was if it is possible not to expand both companions simultaneously when one of them is macro-annotated, but rather stick to moving expansion point. For this snippet a compilation error might be too harsh, because |
Sorry, I’m late to the party, but here’s a random thought: we could make a distinction between “whitebox” and “blackbox” macro-annotations. “blackbox” ones should promise that they do not mess with the declaration names and do not add or remove declarations. Now, everything should be visible for the typechecker, however, referencing whitebox-annotated stuff should produce an error. The tricky thing is of course how to restrict blackbox behavior without having to expand annotations. To the problems:
|
Also I think forward-referencing inside blocks/templates is bad and those who do it should feel bad! |
Well, open recursion is what you get when you sign up for OOP :) |
Can blackbox annotations change signatures of the annottees without changing their names? Could you also give an example of a code snippet that "references whitebox-annotated stuff" and an error that it produces? |
@black val x = 8 + "Y.z"
@white object Y { val z: Int = 4 } where |
What about a compiler plugin then? :) (I am no expert on these) |
What about a compiler plugin? |
@xeno-by I meant doing |
Without dirty hacks, compiler plugins can't change how typer works (with dirty hacks that's going to be roughly what an annotation-based solution is, I think). Analyzer plugins can do that, but they can only postprocess typechecking results, not change typechecking completely, like we need to do here. |
Hey everyone, I want to analyse the type of a wrapped class (using the pimp my library pattern) in order to generate some methods.
Currently I use the |
@b-studios So far there hasn't been much progress unfortunately - this is quite a tricky issue. Could you elaborate on your use case (a standalone github project would be ideal), so that we could possibly come up with a workaround? |
@xeno-by The usecase in short: I use the Thanks alot |
@b-studios Thanks for taking time to publish the reproduction! I'll try to take a look today/tomorrow. |
@xeno-by In order to save you some time I extracted the problem inside of a new branch. This way it is only two short files to take a look at: macrodefinition and usage |
@b-studios The first problem is indeed scalamacros/paradise#14, and you'll have to reformulate the affected code at the moment. How much of a blocker is that to you? The second problem, I think, could be successfully handled within the current framework. How about crafting a more elaborate tree for c.typeCheck, something like |
Also Happy New Year for everyone checking out this thread right now :) |
@xeno-by The type alias issue is not so much of a problem right now, since I currently have influence on the client code. Regarding the type parameter problem: Great idea to bind the missing parameter in the crafted type tree. I will try to use this approach, but I fear it is only applicable if the binding instance of |
@b-studios Could you elaborate on potential problems with the proposed approach for handling type parameters? |
@xeno-by In the example the macro is applied in the context of the class definition where the type parameters are bound. This way one could query the type parameters and manually craft a tree - binding those type parameters in order to If the type arguments of the type constructor are bound outside of the annotated class definition it might be difficult to find the binding occurrence (one would have to reimplement the lookup of types). Please correct me, I just started using scala macros and I don't know how such a lookup could be implemented. I'm happy if I am just wrong with this assumption. Since I perform the typecheck to retreive the If the type tree is an |
@b-studios Oh wow, this is something that I didn't expect:
Prints members of String, not Int. Yes, that's a problem - I'll try to fix it. |
@b-studios Yes, the type of the returned tree will be |
@b-studios I have fixed the problem with typecheck not seeing type parameters declared in enclosing classes. In the morning (in ~8-10 hours), if all the builds are green, I'll publish 2.0.0-M2 with the fix. |
@b-studios 2.0.0-M2 is now published for 2.10.3 and 2.11.0-M7. Please let me know whether it solves the problem with type parameters. |
@xeno-by Sorry for the late message. Thank you alot! Your fix actually works perfectly for member annotations like trait Foo[T] {
@annotation def foo: T
} I guess there is a reason for the constructor argument annotations not to work? class Foo[T](@annotation foo: Bar[T])
// scala.reflect.macros.TypecheckException: not found: type T
I tried this, but extracting |
|
@xeno-by Ad 2) I was using |
Hi,
Do you have any plans for migration from now deprecated untyped macros? I am not still sure if this is going to work, but maybe one could provide
implicit
views fromMonad[A]
/Functor[A]
toA
(e.g. with a@compileTimeOnly
annotation), so that the code typechecks, and the macro would just disregard them, replacing withmap
s orbind
s. What do you think?Nick
The text was updated successfully, but these errors were encountered: