Closed
Bug 757690
Opened 13 years ago
Closed 13 years ago
Merge TokenStream::TokenStream() and TokenStream::init()
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(1 file)
18.31 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
TokenStream has an init() that cannot fail and therefore isn't necessary.
This patch:
- Merges TokenStream::init() with TokenStream::TokenStream().
A knock-on effect is that Parser::Parser gets five more arguments, and
Parser::init() gets five fewer.
- Un-defaults five (yes, five!) arguments in Parser::Parser(). That many
default arguments gives me with willies; explicit is better than
implicit, etc, etc.
I suspect some of the places that use |foldConstants = true| don't need it
-- that's what happens when you overdo the default args -- but I'm being
conservative and not changing them for now.
- Inits Parser::tempPoolMark in the constructor just to be safe.
- Moves the setting of tokenStream.setStrictMode() based on
JSOPTION_STRICT_MODE from Parser::init() to TokenStream::TokenStream(), a
much better place for it.
- Merges TokenStream::TokenBuf::{init,TokenBuf}. The split is no longer
needed.
- Removes a bunch of out-of-date comments from above
TokenStream::TokenStream's declaration -- TokenStream::close no longer
exists, and TokenStream doesn't use JSContext::tempLifoAlloc, AFAICT.
- Adds a missing failure check after the parser.init() call in
shell/js.cpp.
Attachment #626277 -
Flags: review?(jorendorff)
![]() |
||
Comment 1•13 years ago
|
||
Comment on attachment 626277 [details] [diff] [review]
patch
Nice cleanup.
In Parse in shell/js.cpp:
>+ "<string>", /* lineno = */ 0, cx->findVersion(),
While you're in here, please change the lineno to 1.
(0 is always wrong for lineno. It's a very common mistake.)
Attachment #626277 -
Flags: review?(jorendorff) → review+
![]() |
Assignee | |
Comment 2•13 years ago
|
||
> >+ "<string>", /* lineno = */ 0, cx->findVersion(),
>
> While you're in here, please change the lineno to 1.
>
> (0 is always wrong for lineno. It's a very common mistake.)
In conjunction with "<string>" as the filename, I thought this signified "the line number doesn't mean anything". But I'm happy to change this one, and the similar one earlier in the file.
![]() |
Assignee | |
Comment 3•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2105611f8968
I forgot the lineno=1 change, I did it in a follow-up:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c84b9365f6fa
Target Milestone: --- → mozilla15
![]() |
||
Comment 4•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
![]() |
||
Comment 5•13 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•