Closed Bug 444846 Opened 12 years ago Closed 12 years ago

SM: Fixing GCC warning about redefined HAVE_VA_LIST_AS_ARRAY

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: igor, Assigned: igor)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #436263 +++

Currently on Linux-x64 during FireFox build the compiler generates for each SpiderMonkey source file:

./jsautocfg.h:54:1: warning: "HAVE_VA_LIST_AS_ARRAY" redefined
In file included from <command line>:1:
./../../mozilla-config.h:63:1: warning: this is the location of the previous definition
,

The reason for this is that mozilla-config.h also defines the macro on Linux x64. To avoid this clash I suggest to rename HAVE_VA_LIST_AS_ARRAY into JS_HAVE_VA_LIST_AS_ARRAY
Is there a reason we can't simply not define it, if it already exists?
Renaming the bug to reflect the problem, not one among many possible solutions.
Summary: SM: Renaming HAVE_VA_LIST_AS_ARRAY into JS_HAVE_VA_LIST_AS_ARRAY → SM: Fixing GCC warning about redefined HAVE_VA_LIST_AS_ARRAY
Attached patch v1 (obsolete) — Splinter Review
The patch delegates to mozilla-config.h the task of defining HAVE_VA_LIST_AS_ARRAY under Mozilla's build.
Attachment #330098 - Flags: review?
Comment on attachment 330098 [details] [diff] [review]
v1

Grabbing the r?, since you didn't specify.  This looks good to me.
Attachment #330098 - Flags: review? → review+
Though, a perhaps less mozilla-centric view might be to do:

+    printf("#ifndef HAVE_VA_LIST_AS_ARRAY\n");
.... and so on....


instead.
The warning happens because mozilla-config.h and jsautocfg.h define the macro differently:

#define HAVE_VA_LIST_AS_ARRAY 1
versus
#define HAVE_VA_LIST_AS_ARRAY

So the patch simply makes jsautocfg.h to use the same form as mozilla-config.h.
Attachment #330098 - Attachment is obsolete: true
Attachment #330127 - Flags: review?
Comment on attachment 330127 [details] [diff] [review]
avoiding mozilla-config special casing

Also works for me, but I still don't understand why we don't just test #ifndef HAVE_VA_LIST_AS_ARRAY, so that this breakage doesn't happen if/when the external definition changes later.
Attachment #330127 - Flags: review? → review+
(In reply to comment #7)
> (From update of attachment 330127 [details] [diff] [review])
> Also works for me, but I still don't understand why we don't just test #ifndef
> HAVE_VA_LIST_AS_ARRAY, so that this breakage doesn't happen if/when the
> external definition changes later.

HAVE_VA_LIST_AS_ARRAY should have the same meaning when used. Having the same definition helps to check for that and make sure that if somebody defines mistakenly HAVE_VA_LIST_AS_ARRAY as 0, he would get an early warning.
I pushed the fix: http://hg.mozilla.org/mozilla-central/index.cgi/rev/60245c8c0ff3
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Recording the bug dependency.
Blocks: 436263
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.