517990 - NewNameNode, NewBindingNode take unused arguments

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

Attachment: Remove unused 'TS' parameter from NewNameNode and NewBindingNode.

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.

Comment 1

[Brendan Eich [:brendan]]

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

[Brendan Eich [:brendan]]

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

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

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.

Comment 4

[Jim Blandy :jimb]

Followup bug for comment 1 filed, bug 518055.

Comment 5

[Jim Blandy :jimb]

http://hg.mozilla.org/tracemonkey/rev/04d47f16e93c

Comment 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/04d47f16e93c