Last Comment Bug 695097 - Split up frontend/Parser.{h,cpp}
: Split up frontend/Parser.{h,cpp}
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86 Mac OS X
-- normal (vote)
: mozilla10
Assigned To: Jason Orendorff [:jorendorff]
: Jason Orendorff [:jorendorff]
Depends on: 695094
  Show dependency treegraph
Reported: 2011-10-17 12:15 PDT by Jason Orendorff [:jorendorff]
Modified: 2011-10-18 05:39 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

v1 (1.11 MB, patch)
2011-10-17 12:20 PDT, Jason Orendorff [:jorendorff]
luke: review+
Details | Diff | Splinter Review

Description User image Jason Orendorff [:jorendorff] 2011-10-17 12:15:55 PDT
This breaks out three smallish parts of frontend/Parser.{h,cpp}:

  - BytecodeCompiler.{h,cpp}    (the facade, js::Compiler)
  - ParseNode.{h,cpp}           (AST stuff: JSParseNode, JSDefinition, etc.)
  - FoldConstants.{h,cpp}       (js_FoldConstants)

Later, I'll rename the classes and functions into alignment with the filenames.
It's BytecodeCompiler.h rather than Compiler.h because we already have
Comment 1 User image Jason Orendorff [:jorendorff] 2011-10-17 12:20:02 PDT
Created attachment 567529 [details] [diff] [review]

I used "hg cp Parser.cpp ..." so that history is preserved and the patch consists mostly of - lines with very few + lines. Unfortunately that makes the patch super fragile. I made the patch based on some other patches in progress, so (I just realized) it almost certainly won't apply against tip. :-P  If you'd like one that does, that's ok -- I'll be producing one today anyway. But I think this is reviewable as-is.
Comment 2 User image Luke Wagner [:luke] 2011-10-17 12:37:32 PDT
Comment on attachment 567529 [details] [diff] [review]

I agree with your high-level goal and using cp to preserve history.  My one request is that you update the #includes to match the style:
Comment 3 User image Jason Orendorff [:jorendorff] 2011-10-17 15:00:29 PDT
Fixed and pushed.
Comment 4 User image Jason Orendorff [:jorendorff] 2011-10-17 15:51:58 PDT
Requires this follow-up:

Otherwise you get "Symbol undefined" linker errors. The -inl.h file contains the definition of the relevant template, so you need it in order to instantiate the method.

It worked fine in my local DEBUG builds because there is DEBUG-only code in other files that instantiates the same method. I'm not sure why it failed to link in the "debug" builds on the tinderboxen, but I noticed that we are passing both --enable-debug and --enable-optimize to configure.

Note You need to log in before you can comment on or make changes to this bug.