Split up frontend/Parser.{h,cpp}

RESOLVED FIXED in mozilla10

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

Other Branch
mozilla10
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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
js/src/methodjit/Compiler.h.
(Assignee)

Comment 1

6 years ago
Created attachment 567529 [details] [diff] [review]
v1

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.
Assignee: general → jorendorff
Attachment #567529 - Flags: review?(luke)

Comment 2

6 years ago
Comment on attachment 567529 [details] [diff] [review]
v1

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: https://wiki.mozilla.org/JavaScript:SpiderMonkey:C%2B%2B_Coding_Style#Includes.
Attachment #567529 - Flags: review?(luke) → review+
(Assignee)

Comment 3

6 years ago
Fixed and pushed.

http://hg.mozilla.org/integration/mozilla-inbound/rev/31b5cad155fb
(Assignee)

Comment 4

6 years ago
Requires this follow-up:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e3f7294169d

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.
https://hg.mozilla.org/mozilla-central/rev/31b5cad155fb
https://hg.mozilla.org/mozilla-central/rev/9e3f7294169d
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.