401298 - Including jsapi.h generates many warnings with certain compiler configurations (e.g. gcc 3.4 -Wstrict-prototypes)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Wesley W. Garland]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8
Build Identifier: jsapi.h head 3.162

jsapi.h is full if function prototypes with empty arguments.  

While an empty argument list in C++ means "no arguments", in C it means "any number of arguments" (think back to K&R declarations). In C89 and C99, this syntax is deprecated and generates warnings when using gcc -Wstrict-prototypes and almost certainly other C89 or C99 compilers in verbose-warning modes.

The general solution is to replace all function prototypes with empty arguments lists with function prototypes with EMPTY argument lists. i.e.

int myfunc();

becomes

int myfunc(void);

This is conformant C89, C99 and C++98.

http://david.tribble.com/text/cdiffs.htm

[C99: §6.7.5.3]
[C++98: §8.3.5, C.1.6.8.3.5]

Reproducible: Always

Steps to Reproduce:
1. Embed spidermonkey in a C program
2. Compile your program with gcc -Wall -Wstrict-prototypes  
3. Observe the warning messages


Expected Results:  
Built without warnings.

Comment 1

[Wesley W. Garland]

Attachment: Unified Diff of proposed patch against revision 3.162

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

This seems reasonable.

Comment 3

[Brendan Eich [:brendan]]

Who will land? I've whinged before about not putting JS_STATIC_ASSERTS in header files -- it's a modularity killer.

/be

Comment 4

[Brendan Eich [:brendan]]

(In reply to comment #3)
> Who will land? I've whinged before about not putting JS_STATIC_ASSERTS in
> header files -- it's a modularity killer.

Sorry, this was for bug 401300.

/be

Comment 5

[Reed Loden [:reed]]

I'm not sure why this bug is checkin-needed. There aren't any reviews on the patch at all, and it does not have approval. Am I missing something?

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 286333 [details] [diff] [review]
Unified Diff of proposed patch against revision 3.162

Reed, I thought Blake had stamped his approval.

/be

Comment 7

[Reed Loden [:reed]]

Please re-add the checkin-needed keyword once the patch has appropriate review(s) and approval(s).

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 286333 [details] [diff] [review]
Unified Diff of proposed patch against revision 3.162

Sorry, I meant to but got distracted.

Comment 9

[Brendan Eich [:brendan]]

Reed, would it have killed you to have left the keyword set for a few hours? Prolly :-P.

/be

Comment 10

[Reed Loden [:reed]]

Uh, Brendan, this still can't be checked-in. This doesn't have approval at all, or is this NPOTB?

Comment 11

[Reed Loden [:reed]]

Ok, I'll land this as soon as the tree reopens for general check-ins, unless you want to try to get this in before M9 (http://wiki.mozilla.org/TreeStatus).

Comment 12

[Reed Loden [:reed]]

Checking in js/src/jsapi.h;
/cvsroot/mozilla/js/src/jsapi.h,v  <--  jsapi.h
new revision: 3.163; previous revision: 3.162
done