Closed Bug 850074 Opened 7 years ago Closed 7 years ago

Move js/src/gc/Root.h to js/public/RootingAPI.h

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(1 file)

Attached patch v0Splinter Review
We want to be able to use this in the browser without pulling in all of jsapi.h.
Attachment #723724 - Flags: review?(wmccloskey)
Comment on attachment 723724 [details] [diff] [review]
v0

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

Can you make sure that #include "js/..." always comes after any #include "js..." declarations, in the same block as #include "ion/..." and the like? I think that's where they're supposed to go, at least.

::: js/src/ion/C1Spewer.h
@@ +9,5 @@
>  
>  #ifndef jsion_c1spewer_h__
>  #define jsion_c1spewer_h__
>  
> +#include "js/RootingAPI.h"

This should go after jsscript.h I think.

::: js/src/ion/CompilerRoot.h
@@ +7,5 @@
>  
>  #if !defined(jsion_ion_gc_h__) && defined(JS_ION)
>  #define jsion_ion_gc_h__
>  
> +#include "js/RootingAPI.h"

This should go after jscntxt.h I think.

::: js/src/ion/IonMacroAssembler.cpp
@@ +4,5 @@
>   * This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#include "js/RootingAPI.h"

This should go in the same block as the ion and vm stuff, I think.

::: js/src/ion/JSONSpewer.h
@@ +9,5 @@
>  #define js_ion_jsonspewer_h__
>  
>  #include <stdio.h>
>  
> +#include "js/RootingAPI.h"

Move down

::: js/src/jsapi-tests/tests.cpp
@@ +5,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #include "tests.h"
> +#include "js/RootingAPI.h"

Move down

::: js/src/jscompartment.cpp
@@ +7,5 @@
>  
>  #include "mozilla/DebugOnly.h"
>  
> +#include "js/MemoryMetrics.h"
> +#include "js/RootingAPI.h"

I think this should go below.

::: js/src/jsgcinlines.h
@@ +7,5 @@
>  #ifndef jsgcinlines_h___
>  #define jsgcinlines_h___
>  
> +#include "js/RootingAPI.h"
> +#include "js/TemplateLib.h"

Same.

::: js/src/jsinferinlines.h
@@ +5,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  /* Inline members for javascript type inference. */
>  
> +#include "js/RootingAPI.h"

Same.

::: js/src/jsscript.h
@@ +9,5 @@
>  #define jsscript_h___
>  /*
>   * JS script descriptor.
>   */
> +#include "js/RootingAPI.h"

Same.

::: js/src/vm/PropertyKey.cpp
@@ +13,2 @@
>  #include "js/Value.h"
> +

Don't think we want the blank line.

::: js/src/vm/Shape.h
@@ +11,5 @@
>  #include "mozilla/Attributes.h"
>  #include "mozilla/GuardObjects.h"
>  
> +#include "js/HashTable.h"
> +#include "js/RootingAPI.h"

Same.

::: js/src/vm/String.h
@@ +11,5 @@
>  #include "mozilla/Attributes.h"
>  #include "mozilla/GuardObjects.h"
>  
>  #include "js/CharacterEncoding.h"
> +#include "js/RootingAPI.h"

Both should be moved down.
Attachment #723724 - Flags: review?(wmccloskey) → review+
(In reply to Bill McCloskey (:billm) from comment #1)
> 
> Can you make sure that #include "js/..." always comes after any #include
> "js..." declarations, in the same block as #include "ion/..." and the like?
> I think that's where they're supposed to go, at least.
 
Uhg, alright. I'll update the style guide too then. It doesn't say in this case, so I assumed js/public would be more like mozilla/ than our random internal stuff that happens to be in sub-directories.
I don't really care how it is as long as it's consistent. In some of the files you did it the other way.
Blocks: 850293
https://hg.mozilla.org/mozilla-central/rev/d537ff6052e8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Depends on: 858539
You need to log in before you can comment on or make changes to this bug.