https://codereview.chromium.org/749633002/diff/80001/src/compiler/ast-graph-builder.cc
File src/compiler/ast-graph-builder.cc (right):
https://codereview.chromium.org/749633002/diff/80001/src/compiler/ast-graph-builder.cc#newcode2016
src/compiler/ast-graph-builder.cc:2016: if
(variable->IsSignallingAssignmentToConst(op, strict_mode())) {
On 2014/11/25 17:22:31, Dmitry Lomov (chromium) wrote:
On 2014/11/25 16:25:02, rossberg wrote:
> On 2014/11/25 15:51:01, Dmitry Lomov (chromium) wrote:
> > On 2014/11/25 15:23:36, rossberg wrote:
> > > I'm not sure I like this factorisation, since it breaks the
symmetry with
> the
> > > other cases below and thereby makes the code less readable. It
also
> duplicates
> > > some of the cases analysis. Better factor it out into the two
relevant
cases
> > > below.
> > >
> > > Michi, what do you think?
> >
> > I'd prefer to share logic with the backends. Michi?
>
> Well, sharing the logic would make sense if we did so consistently
for the
rest
> of this branching logic as well. But arbitrarily picking one case
for a remote
> helper function is "spurious abstraction" -- which is harmful
because it
> pretends there is something special going on when there isn't.
>
> So unless you think there is benefit in also introducing
> IsSignallingTheHoleAssignment, IsSilentlyIgnoredAssignment and
> IsSilentlyIgnoredButSignallingTheHoleAssignment, you don't want to
introduce
> this one. ;)
Yes I am quite sure there is a benefit in introducing all these.
Otherwise we
repeat the same logic in more than 5 places!
I can do this in this CL or in subsequent CL.
My point was: you could argue the same for a gazillion of other pieces
of logic in the compilers.
The other backends are just one gigantic, obsolete mess of wholesale
code duplication anyway, and have always been. Let's not engage in
futile attempts to micro-fight that by regressing the clarity of the new
and shiny TF code base. Our decided route to getting rid of the
duplication is a clean slate with Turbofan. That makes distributing
random tidbits of logic to random helpers in random places
counterproductive. Because ultimately, the logic should all be in here
and only here. ;)
https://codereview.chromium.org/749633002/
--
--
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.
