Closed Bug 757690 Opened 13 years ago Closed 13 years ago

Merge TokenStream::TokenStream() and TokenStream::init()

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

Attachments

(1 file)

Attached patch patchSplinter 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 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+
> >+ "<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.
Target Milestone: --- → mozilla15
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: