Last Comment Bug 695094 - Move js{scan,parse,emit}.{h,cpp} into js/src/frontend/
: Move js{scan,parse,emit}.{h,cpp} into js/src/frontend/
Status: RESOLVED FIXED
:
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]
Mentors:
Depends on:
Blocks: 695097
  Show dependency treegraph
 
Reported: 2011-10-17 12:10 PDT by Jason Orendorff [:jorendorff]
Modified: 2011-10-18 05:39 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

Description Jason Orendorff [:jorendorff] 2011-10-17 12:10:27 PDT

    
Comment 1 Jason Orendorff [:jorendorff] 2011-10-17 12:12:17 PDT
Created attachment 567523 [details] [diff] [review]
v1
Comment 2 Luke Wagner [:luke] 2011-10-17 12:29:41 PDT
Comment on attachment 567523 [details] [diff] [review]
v1

Review of attachment 567523 [details] [diff] [review]:
-----------------------------------------------------------------

The style for header include order and spacing is not universal, but we seem to have converged which cdleary has documented: https://wiki.mozilla.org/JavaScript:SpiderMonkey:C%2B%2B_Coding_Style#Includes.  Based on that, style nits below:

::: js/src/jsemit.cpp
@@ +61,5 @@
>  #include "jsscope.h"
>  #include "jsscript.h"
> +#include "frontend/CodeGenerator.h"
> +#include "frontend/Parser.h"
> +#include "frontend/TokenStream.h"

spaces before/after and go after jsautooplen.h

::: js/src/jsparse.cpp
@@ +77,5 @@
>  #include "jsstr.h"
>  #include "jslibmath.h"
> +#include "frontend/CodeGenerator.h"
> +#include "frontend/Parser.h"
> +#include "frontend/TokenStream.h"

spaces before, and after the jsxml/jsdhash

::: js/src/jsatom.cpp
@@ +60,4 @@
>  #include "jsstr.h"
>  #include "jsversion.h"
>  #include "jsxml.h"
> +#include "frontend/Parser.h"

space before

::: js/src/jsfun.cpp
@@ +68,5 @@
>  #include "jsexn.h"
>  #include "jstracer.h"
> +#include "frontend/CodeGenerator.h"
> +#include "frontend/Parser.h"
> +#include "frontend/TokenStream.h"

space before (preexisting)

::: js/src/jsobj.cpp
@@ +77,5 @@
>  #include "json.h"
>  #include "jswatchpoint.h"
>  #include "jswrapper.h"
> +#include "frontend/CodeGenerator.h"
> +#include "frontend/Parser.h"

space before

::: js/src/json.cpp
@@ +57,5 @@
>  #include "jstypes.h"
>  #include "jsstdint.h"
>  #include "jsutil.h"
>  #include "jsxml.h"
> +#include "frontend/TokenStream.h"

space before

::: js/src/jsscript.cpp
@@ +65,5 @@
>  #include "jstracer.h"
>  #if JS_HAS_XDR
>  #include "jsxdrapi.h"
>  #endif
> +#include "frontend/CodeGenerator.h"

space before

::: js/src/shell/js.cpp
@@ +74,5 @@
>  #include "jstypedarray.h"
>  #include "jstypedarrayinlines.h"
>  #include "jsxml.h"
>  #include "jsperf.h"
> +#include "frontend/CodeGenerator.h"

space before

::: js/src/vm/Debugger.cpp
@@ +41,5 @@
>  
>  #include "vm/Debugger.h"
>  #include "jsapi.h"
>  #include "jscntxt.h"
> +#include "frontend/CodeGenerator.h"

moved to the end with spaces (along with several other #includes)

::: js/src/vm/GlobalObject.cpp
@@ +44,5 @@
>  #include "jsexn.h"
>  #include "jsmath.h"
>  #include "json.h"
> +#include "builtin/RegExp.h"
> +#include "frontend/CodeGenerator.h"

space before
Comment 3 Jason Orendorff [:jorendorff] 2011-10-17 14:57:58 PDT
Good catch on the spacing. Fixed and pushed.

http://hg.mozilla.org/integration/mozilla-inbound/rev/28fcc7211b70
Comment 4 Marco Bonardo [::mak] 2011-10-18 05:39:10 PDT
https://hg.mozilla.org/mozilla-central/rev/28fcc7211b70

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