Closed Bug 398946 Opened 12 years ago Closed 11 years ago

Remove JS_STATIC_DLL_CALLBACK and JS_DLL_CALLBACK from the tree

Categories

(Core :: JavaScript Engine, defect, trivial)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla1.9.1b1

People

(Reporter: mats, Assigned: Swatinem)

References

()

Details

Attachments

(3 files, 3 obsolete files)

JS_STATIC_DLL_CALLBACK translates to "static" and JS_DLL_CALLBACK to nothing
for all platforms except #ifdef WIN16:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/js/src/jstypes.h&rev=3.42&root=/cvsroot&mark=86-87,96-97,107-108,116-117,133-134#79
As far as I know, WIN16 is only defined for Windows 3.1 or older - 
which we don't support.
Attached patch completely remove those macros (obsolete) — Splinter Review
The #ifdef WIN16 has been removed for some time.
This patch completely removes those macros replacing them with static or nothing respectively.
Assignee: general → arpad.borsos
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #333413 - Flags: superreview?(brendan)
Attachment #333413 - Flags: review?(brendan)
Comment on attachment 333413 [details] [diff] [review]
completely remove those macros

Benjamin, I could use your help here -- I'm deep in other commitments. Thanks,

/be
Attachment #333413 - Flags: superreview?(brendan)
Attachment #333413 - Flags: superreview?(benjamin)
Attachment #333413 - Flags: review?(brendan)
Attachment #333413 - Flags: review?(benjamin)
Comment on attachment 333413 [details] [diff] [review]
completely remove those macros

I'm fine with this if brendan is!
Attachment #333413 - Flags: superreview?(benjamin)
Attachment #333413 - Flags: review?(benjamin)
Attachment #333413 - Flags: review+
Yeah, get rid of unneeded macros.

/be
Thanks for the review. checkin-needed
Keywords: checkin-needed
Comment on attachment 333413 [details] [diff] [review]
completely remove those macros

This patch breaks with the new worker threads feature. I will post an updated patch shortly.
Attachment #333413 - Attachment is obsolete: true
Attached patch updated for worker threads code (obsolete) — Splinter Review
Attachment #334267 - Flags: review?(brendan)
Comment on attachment 334267 [details] [diff] [review]
updated for worker threads code

Ben should do the r+ then.

/be
Attachment #334267 - Flags: review?(brendan) → review?(bent.mozilla)
An interdiff between two diff-able patches would be great.

/be
Attached patch interdiffSplinter Review
Comment on attachment 334267 [details] [diff] [review]
updated for worker threads code

>-  static JSBool JS_DLL_CALLBACK AddEventListenerHelper(JSContext *cx,
>+  static JSBool AddEventListenerHelper(JSContext *cx,
>                                                        JSObject *obj,
>                                                        uintN argc, jsval *argv,
>                                                        jsval *rval);

> typedef JSBool
>-(* JS_DLL_CALLBACK JSDHashInitEntry)(JSDHashTable *table,
>+(* JSDHashInitEntry)(JSDHashTable *table,
>                                      JSDHashEntryHdr *entry,
>                                      const void *key);

Things like this make the code really ugly... If you're going to change the indentation of the first line then please rewrap the next ones.
Attached patch indentation fixed (obsolete) — Splinter Review
Attachment #334267 - Attachment is obsolete: true
Attachment #334335 - Flags: review?(bent.mozilla)
Attachment #334267 - Flags: review?(bent.mozilla)
I found a typo which stopped the patch from compiling, sorry.
Attachment #334335 - Attachment is obsolete: true
Attachment #334449 - Flags: review?(bent.mozilla)
Attachment #334335 - Flags: review?(bent.mozilla)
Comment on attachment 334449 [details] [diff] [review]
fixed a typo
[Checkin: Comment 15]

Thanks! Assuming this compiles and passes unit tests I think this looks fine.
Attachment #334449 - Flags: review?(bent.mozilla) → review+
Keywords: checkin-needed
Comment on attachment 334449 [details] [diff] [review]
fixed a typo
[Checkin: Comment 15]

Fixed 1 "rej" from
[
patching file dom/src/base/nsJSEnvironment.cpp
Hunk #4 succeeded at 871 with fuzz 1 (offset 12 lines).
patching file js/src/jsapi.cpp
Hunk #1 FAILED at 4335
1 out of 2 hunks FAILED -- saving rejects to file js/src/jsapi.cpp.rej
patching file js/src/jscntxt.cpp
Hunk #1 succeeded at 84 with fuzz 2 (offset 4 lines).
patching file js/src/jscntxt.h
Hunk #1 succeeded at 195 with fuzz 2 (offset 62 lines).
patching file js/src/jsxml.cpp
Hunk #1 succeeded at 6038 with fuzz 2 (offset -146 lines).
patching file js/src/jsxml.h
Hunk #1 succeeded at 56 with fuzz 1 (offset -54 lines).
patching file js/src/liveconnect/jsj_JavaClass.c
Hunk #1 succeeded at 60 with fuzz 2 (offset -1 lines).
patching file js/src/prmjtime.cpp
Hunk #1 succeeded at 561 with fuzz 2 (offset 0 lines).
patching file js/src/xpconnect/src/xpcconvert.cpp
Hunk #1 succeeded at 167 with fuzz 2 (offset -45 lines).
]

http://hg.mozilla.org/mozilla-central/rev/e71240d4b28c
Attachment #334449 - Attachment description: fixed a typo → fixed a typo [Checkin: Comment 15]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
[
dom_quickstubs.cpp:117: error: ‘JS_DLL_CALLBACK’ does not name a type
...
dom_quickstubs.cpp:12207: error: invalid use of non-lvalue array
]

Update new <qsgen.py> too.
http://hg.mozilla.org/mozilla-central/rev/b4ccb313bbb7
You need to log in before you can comment on or make changes to this bug.