Closed Bug 391423 Opened 13 years ago Closed 13 years ago

Reposition JS_(BEGIN|END)_EXTERN_C to avoid nesting #includes

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: Mardak, Assigned: Mardak)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch hg/actionmonkey patch (obsolete) — Splinter Review
This causes problems for MMgc and C++ stuff that end up in extern "C" {} blocks.

Attached is a patch for hg/actionmonkey, but the idea would work on trunk as well.
Attachment #275825 - Flags: review?(jorendorff)
Attached patch cvs v1Splinter Review
Attachment #275825 - Attachment is obsolete: true
Attachment #275830 - Flags: review?(jorendorff)
Attachment #275825 - Flags: review?(jorendorff)
Comment on attachment 275830 [details] [diff] [review]
cvs v1

The patch works for hg/actionmonkey w/ MMgc, but the minimal changes needed is the following:

>+++ js/src/jslock.h	8 Aug 2007 20:17:10 -0000
>@@ -107,17 +107,19 @@ typedef struct JSFatLockTable {
> /*
>  * Include jsscope.h so JS_LOCK_OBJ macro callers don't have to include it.
>  * Since there is a JSThinLock member in JSScope, we can't nest this include
>  * much earlier (see JSThinLock's typedef, above).  Yes, that means there is
>  * an #include cycle between jslock.h and jsscope.h: moderate-sized XXX here,
>  * to be fixed by moving JS_LOCK_SCOPE to jsscope.h, JS_LOCK_OBJ to jsobj.h,
>  * and so on.
>  */
>+JS_END_EXTERN_C
> #include "jsscope.h"
>+JS_BEGIN_EXTERN_C
> 
> #define JS_LOCK_RUNTIME(rt)         js_LockRuntime(rt)
> #define JS_UNLOCK_RUNTIME(rt)       js_UnlockRuntime(rt)
Attachment #275830 - Flags: review?(benjamin)
Attachment #275830 - Flags: review?(jorendorff) → review+
It'd be nice to get this patch into trunk.  I'd be happy if you could check the minimal change into actionmonkey, though.  We need it: Firefox won't even build in actionmonkey right now.  And I think if an identical change later comes downstream from trunk, hg will merge correctly without protest.

Attachment #275830 - Flags: review?(benjamin) → review+
Can someone check this in to cvs? I'll merge hg/cvs-trunk-mirror to hg/mozilla-central then that to hg/actionmonkey.
Keywords: checkin-needed
In on trunk.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M8
Thanks waldo for checking in to cvs.
I've pulled to hg/mozilla-central changeset 4486:  	db456f3a9ed8
and merged into hg/actionmonkey changeset 4519:  	db456f3a9ed8
Target Milestone: mozilla1.9 M8 → ---
You need to log in before you can comment on or make changes to this bug.