On 2015/11/20 20:05:41, caitp wrote:
On 2015/11/20 20:02:24, adamk wrote:
> On 2015/11/20 19:56:45, caitp wrote:
> > On 2015/11/20 19:49:48, adamk wrote:
> > > Having read through a few times, I have some high-level questions:
> > >
> > > - You mentioned on IRC that the
keeping-track-of-assigments-to-rewrite
thing
> > > didn't interact well with something else. Was that resolved?
Overall, do
you
> > > know of any cases where this doesn't work correctly?
> > >
> >
> > I believe that was resolved --- the last issues are the ones
mentioned in
my
> > previous comment.
>
> Ah, I didn't see that comment (didn't get emailed to me for some
reason).
>
> The stress-opt problem sounds possibly bad, I'd be interested in more
> investigation there. Note that there has been some recent churn in
TurboFan
> support for spread.
>
> > > - It seems a bit strange that the AssignmentPattern AST node
subsumes
the
> > entire
> > > Assignment. This makes the code in full-codegen look odd, since
only the
> > target
> > > is visited and not the value. Was there a reason you didn't put
more of
this
> > > logic in Assignment?
> >
> > The AssignmentPattern node is basically a stand-in for the rewritten
> expression.
> > It's unfortunate that it's done this way, but since the Parser is
doing
the
> > desugaring, I'm not sure of a better way to do it.
>
> But why isn't this handled by the Assignment node, rather than being
buried
down
> in the target? It's really the whole Assignment that gets rewritten, not
just
> the target.
Because, there isn't really a way to go in and completely replace the
Assignment
with the rewritten version, since there's no way to access the parent
node and
insert the replacement into the right place --- so, instead, the LHS is a
stand-in for the rewritten Assignment. It's a hack, but I'm not sure
there's a
better way here, given that the parser is doing the desugaring
Per discussion on IRC, you seem to be saying that you'd prefer storing the
entire Assignment in the list, and storing the rewritten expression in all
Assignment nodes --- is that right? But you'd still have to, in the
compiler,
avoid doing the normal Assignment node work.
To me, either way it seems like a horrible hack, and either way it's going
to be
fixed later once it's possible. I can make the change if that's preferred,
but
it's not a significant difference, and still needs little hacks added to
each
full-codegen implementation
https://codereview.chromium.org/1309813007/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
