528263 - Hide warnings: "warning: anonymous variadic macros were introduced in C99"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

I get these warnings whenever I build mozilla-central (using g++ 4.3):

> /mozilla/js/src/nanojit/nanojit.h:69:22: warning: anonymous variadic macros were introduced in C99
> /mozilla/js/src/nanojit/nanojit.h:70:26: warning: anonymous variadic macros were introduced in C99
> /mozilla/js/src/nanojit/nanojit.h:115:54: warning: anonymous variadic macros were introduced in C99
> /mozilla/js/src/nanojit/nanojit.h:121:53: warning: anonymous variadic macros were introduced in C99
> /mozilla/js/src/nanojit/nanojit.h:123:36: warning: anonymous variadic macros were introduced in C99
> /mozilla/js/src/nanojit/nanojit.h:171:26: warning: anonymous variadic macros were introduced in C99
> /mozilla/js/src/nanojit/Native.h:53:27: warning: anonymous variadic macros were introduced in C99
> /mozilla/js/src/nanojit/Native.h:54:23: warning: anonymous variadic macros were introduced in C99
> /mozilla/js/src/nanojit/Native.h:118:28: warning: anonymous variadic macros were introduced in C99

These warnings refer to macros introduced in bug 514537 and bug 517328.  I assume we don't care that this feature was introduced in C99 (or if we do care, we wouldn't be using it), so these warnings are just needless compiler-spam.

I propose that we use the "-Wno-variadic-macros" flag for g++, to hide these warnings.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix: use -Wno-variadic-macros

Comment 2

[Graydon Hoare :graydon]

umm .. sure, but look a few lines down in the configury: it's already testing for whether or not your compiler supports -Wno-variadic-macros and setting that flag if your gcc understands it. Is that not working?

Comment 3

[Daniel Holbert [:dholbert]]

Ah, good point -- that is "working" in the sense that it correctly detects that my compiler supports it...

>  checking whether the compiler supports -Wno-variadic-macros... yes

but it's not "working" in the sense that that argument doesn't end up getting used, apparently (since I still get the warnings in a stock mozilla-central build).

Investigating this a bit more, will report back in a bit.

Comment 4

[Daniel Holbert [:dholbert]]

Aha! So, a few things:
 - The attached patch doesn't actually fix the warnings -- but it does if I apply its change to /mozilla/configure.in instead of /mozilla/js/src/configure.in
 - The g++ commands in js/src all do include "-Wno-variadic-macros", so graydon's right that js/src/configure.in handles this fine already.
 - The compile command that causes the warnings (all of them) is for "dom_quickstubs.cpp", which is apparently generated from dom_quickstubs.qsconf, and which gets built separately from js/src (and hence doesn't benefit from its -Wno-variadic-macros)

So, to fix this, I think we need -Wno-variadic-macros in /mozilla/configure.in.  In particular, we can probably just copy over the chunk from JS's configure.in that Graydon mentioned in comment 2.

Comment 5

[Daniel Holbert [:dholbert]]

Comment on attachment 412013 [details] [diff] [review]
fix: use -Wno-variadic-macros

(In reply to comment #4)
>  - The attached patch doesn't actually fix the warnings

(Sorry about that -- I'd started with a patch that added "-Wno-variadic-macros" to both /mozilla/configure.in and /mozilla/js/src/configure.in, which *did* fix the warnings, and then I removed the changes to the top-level configure.in file because I'd assumed that the js one was all that was needed.  The rebuild that proved my assumption wrong wasn't complete until after I posted the patch, though.)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: fix: copy "-Wno-variadic-macros" chunk from js's configure.in to top-level one

This copies the chunk that Graydon referred to in comment 2 to the corresponding spot in the top-level configure.in file.  Testing it locally to be absolutely sure it fixes the warnings...

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 412041 [details] [diff] [review]
fix: copy "-Wno-variadic-macros" chunk from js's configure.in to top-level one

Yup, it fixes the warnings. Requesting review.

Comment 8

[Graydon Hoare :graydon]

Comment on attachment 412041 [details] [diff] [review]
fix: copy "-Wno-variadic-macros" chunk from js's configure.in to top-level one

I don't think I actually have authority to review changes to this file. Try bsmedberg?

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: fix as landed (bitrot-fixed)

Landed fix:
  http://hg.mozilla.org/mozilla-central/rev/bd31d27c850c

(fixed bitrot & removed some recently-added excess whitespace just below the modified code)