class Bug {
def f(x: Int, y: Int) = x + y
def g(a: Int, b: Int) = f(y = b, x = a)
}
DefDef(
Modifiers(0, , List()), g, List(), List(
List(
ValDef(Modifiers(8192, , List()), a, Ident(Int), EmptyTree()), ValDef(Modifiers(8192, , List()), b, Ident(Int), EmptyTree())
)
), TypeTree(), Apply(
Ident(f), List(AssignOrNamedArg(Ident(y), Ident(b)), AssignOrNamedArg(Ident(x), Ident(a)))
)
)
DefDef(
Modifiers(), TermName("g"), List(), List(
List(
ValDef(
Modifiers(PARAM), TermName("a"), TypeTree().setOriginal(Select(Ident(scala), scala.Int)), EmptyTree
), ValDef(
Modifiers(PARAM), TermName("b"), TypeTree().setOriginal(Select(Ident(scala), scala.Int)), EmptyTree
)
)
), TypeTree(), Block(
List(
ValDef(Modifiers(ARTIFACT), TermName("x$1"), TypeTree(), Ident(TermName("b"))), ValDef(Modifiers(ARTIFACT), TermName("x$2"), TypeTree(), Ident(TermName("a")))
), Apply(
Select(This(TypeName("Bug")), TermName("f")), List(Ident(TermName("x$2")), Ident(TermName("x$1")))
)
)
)
I could not find any useful information about the named parameters in the tree from above after careful inspection, but maybe I did not look close enough.... what are your ideas about this issue? Would it be possible to add an "original" field, where information about named parameters is retained?--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-interna...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Would it be enough to keep the original (untyped) `Apply` node an attachment?
Do we have other examples where the presentation compiler keeps more data around?
BTW, in Scala 2.11, f(x = 1) can mean f({x = 1; ()}), but should be changed in 2.12:
https://groups.google.com/forum/#!topic/scala-internals/MujIVZgbwEM--On Mon, Jul 6, 2015 at 9:43 AM, Mirko Stocker <mi...@stocker.email> wrote:On Sunday 05 July 2015 13.09:21 Som Snytt wrote:
> The workaround might be to use positions to examine the source, reparsing it
> for pleasure.
I've tried this approach, but then had to revert it because it was too slow
IIRC. But the code is still there, in the ApplyExtractor and BlockExtractor
objects in PimpedTrees. The attached patch enables it, and there are even some
@ignored tests in RenameTest.
Cheers
Mirko
--
Mirko Stocker | mi...@stocker.email
Work: http://ifs.hsr.ch | http://infoq.com
Personal: http://misto.ch | http://twitter.com/m_st
--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-interna...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-interna...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
On Mon, Jul 6, 2015 at 11:54 AM, Lukas Rytz <lukas...@gmail.com> wrote:Would it be enough to keep the original (untyped) `Apply` node an attachment?I think that should work.Do we have other examples where the presentation compiler keeps more data around?One example that comes to mind is in TypeTrees (the .original keeps the type selection, if there was one).
On Mon, Jul 6, 2015 at 12:28 PM, iulian dragos <jagu...@gmail.com> wrote:On Mon, Jul 6, 2015 at 11:54 AM, Lukas Rytz <lukas...@gmail.com> wrote:Would it be enough to keep the original (untyped) `Apply` node an attachment?I think that should work.Do we have other examples where the presentation compiler keeps more data around?One example that comes to mind is in TypeTrees (the .original keeps the type selection, if there was one).AFAIK this is not specific to the presentation compiler. For the Apply nodes we shouldmaybe save the overhead in the normal compiler.
I can take a closer look at this at the weekend... still, even if the performance was OK, duplicating parsing logic for something that has just been parsed seems far from ideal to me. Attaching the original tree to the transformed Apply node as discussed below makes much more sense.
--
On Sunday 05 July 2015 13.09:21 Som Snytt wrote:
> The workaround might be to use positions to examine the source, reparsing it
> for pleasure.
I've tried this approach, but then had to revert it because it was too slow
IIRC. But the code is still there, in the ApplyExtractor and BlockExtractor
objects in PimpedTrees. The attached patch enables it, and there are even some
@ignored tests in RenameTest.
We don't have this on our short list right now, but we'd be happy to assist if you'd like to work on it,
and we can provide some pointers how things could be done.
--
> How did the performance problems you mentioned manifest themselves?
Sorry for the late reply, I was on vacation..
I found the original thread after which we disabled the named arguments patch:
https://groups.google.com/forum/#!topic/scala-ide-user/fmTwrowuvHc
Reading it again, we should only do these more expensive checks when renaming,
and not for mark occurrences, then the performance impact shouldn't be that
painful. What do you think?
--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-interna...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Hi
Again, sorry for the slow reply, I (again) was on vacation (the last one this
year :-).
> > and could not see any regressions after enabling the named parameters
> > functionality.
Ok, that sounds good!
> > If we could easily bring your implementation in scala-refactoring back
> > into production, we would have a notably enhanced renaming feature quite
> > soon. Of course that doesn't change the fact that technically, adapting
> > the
> > compiler to attach the needed information to the ASTs, is to be preferred.
>
> I really think that tree attachments are the right way. 2.11.8 is due
> around September, so there's some time to still put this in. Would you like
> to give it a try? Lukas offered to provide some guidance :)
This sounds like a great opportunity to fix the problem at the root, and your
time is probably better spent here than figuring out memory leaks and worrying
about performance regressions. What do you think?