Closed Bug 1529607 Opened 8 months ago Closed 8 months ago

Rewrite NameFunctions.cpp to use ParseNodeVisitor

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: jorendorff, Assigned: jorendorff)

References

Details

Attachments

(7 files)

No description provided.

I tried making ParseNodeVisitor take all nodes as references, but that didn't
work nicely with the existing accept() method templates. That could have been
made to work using more template tricks, but I decided pointers are not so bad.

There still was no way to avoid the code duplication here without contortions.

Depends on D20715

The Expr suffix is for nodes that can appear anywhere an expression could
appear. This kind of node can't; it's always the direct child of a tagged
template.

Depends on D20716

This is meant as a sanity patch. Before, buf_ was a pointer to a local
variable, set in resolveFun() and just left dangling on exit. No bug, but
dangling pointers are bad.

I considered removing buf_ and passing around a reference to the local
StringBuffer, but this was quicker and seemed easier to review.

Depends on D20718

This may lose some tiny amount of performance since the existing code
duplicated a huge amount of code in order to avoid walking bits of the tree
that can't contain functions. I preserved a few of those hacks but some of the
bits seemed too small to bother with.

The expression nparents_ - 1 is changed to nparents_ - 2 because, as a
result of how ParseNodeVisitor control flow works, we now call gatherNameable
after pushing the current FunctionNode to the stack, rather than before.
(A new assertion checks that this is the case.)

Depends on D20719

Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/61a4f82b54a8
Part 1: Fix misleading name in existing ParseNodeVisitor code. r=khyperia
https://hg.mozilla.org/integration/autoland/rev/e7fd0ec90ddf
Part 2: Rename ParseNodeVisitor::cx to cx_. r=khyperia
https://hg.mozilla.org/integration/autoland/rev/546cbdef316a
Part 3: Split out ParseNodeVisitor and RewritingParseNodeVisitor. r=khyperia
https://hg.mozilla.org/integration/autoland/rev/bb3aa942e67a
Part 4: Rename ParseNodeKind::CallSiteObjExpr to CallSiteObj. r=khyperia
https://hg.mozilla.org/integration/autoland/rev/64df804d18da
Part 5: Style tweaks to existing NameFunctions code. r=khyperia
https://hg.mozilla.org/integration/autoland/rev/ca07a2ad70c2
Part 6: Reuse a single StringBuffer in NameResolver. r=khyperia
https://hg.mozilla.org/integration/autoland/rev/06dc7a3404fc
Part 7: Rewrite NameFunctions.cpp to use ParseNodeVisitor. r=khyperia
Assignee: nobody → jorendorff
You need to log in before you can comment on or make changes to this bug.