Closed
Bug 517990
Opened 15 years ago
Closed 15 years ago
NewNameNode, NewBindingNode take unused arguments
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jimb, Assigned: jimb)
Details
Attachments
(1 file)
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
Neither of these functions uses their 'ts' arguments. And since both are allocators, it seems unlikely that they'd need to use the token stream to report an error related to a particular source position.
Attachment #401943 -
Flags: review?(mrbkap)
Comment 1•15 years ago
|
||
The bigger cleanup I kept refraining from doing in the upvar2 work, to keep the patch only insanely big instead of super-insanely big, is to get rid of almost all the parameters to jsparse.cpp functions, making most of them methods of JSCompiler. This should speed up the recursive-descent parser.
Jim: you want to do it? I was going to do it as parasympathetic nervous system therapy some night, but it's fair game for anyone.
Beyond this there are some cleanups to jsscan.h to use C++ canonical style. But again the JSCompiler wants at least some inline helpers to forward calls on itself to its tokenStream member, to save typing and shorten lines full of redundancy in jsparse.cpp.
/be
Comment 2•15 years ago
|
||
To say a bit more, all the C static functions that have the JSParser signature:
typedef JSParseNode *
JSParser(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc);
become JSCompiler methods taking zero args. Any cx uses become free and need to rename to context -- or better, we rename JSCompiler::context -> cx. Similarly for ts -> &tokenStream, mutatis mutandis. The tc param needs to be a top of tree context stack pointer in JSCompiler, but that's easy to do too.
Hope this helps, and actually wins. Zero params has to make a difference on code perf as well as size, even if we sometimes load this->cx or whatever. But a second opinion is welcome.
/be
Assignee | ||
Updated•15 years ago
|
Assignee: general → jim
Comment 3•15 years ago
|
||
Comment on attachment 401943 [details] [diff] [review]
Remove unused 'TS' parameter from NewNameNode and NewBindingNode.
This is fine for now. I think we should file a followup bug on comment 1.
Attachment #401943 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 4•15 years ago
|
||
Followup bug for comment 1 filed, bug 518055.
Assignee | ||
Comment 5•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•