1155393 - mozalloc.h:182:1: error: replacement function 'operator new' cannot be declared 'inline' [-Werror,-Winline-new-delete]

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Daniel Holbert [:dholbert]]

Bug 1062473 added this #include...
>  #include "mozilla/mozalloc.h"
...to js/src/shell/js.cpp.

For some reason, this introduced this set of build warnings (treated as errors in warnings-as-errors builds) in clang 3.6 and clang 3.7:

{
 1:16.05 In file included from $OBJ/js/src/shell/Unified_cpp_js_src_shell0.cpp:11:
 1:16.06 In file included from $SRC/js/src/shell/js.cpp:13:
 1:16.06 ../../../dist/include/mozilla/mozalloc.h:182:1: error: replacement function 'operator new' cannot be declared 'inline' [-Werror,-Winline-new-delete]
 1:16.06 MOZALLOC_INLINE
 1:16.06 ^
 1:16.06 ../../../dist/include/mozilla/mozalloc.h:31:27: note: expanded from macro 'MOZALLOC_INLINE'
 1:16.06 #  define MOZALLOC_INLINE MOZ_ALWAYS_INLINE_EVEN_DEBUG
 1:16.06                           ^
 1:16.06 ../../../dist/include/mozilla/Attributes.h:27:75: note: expanded from macro 'MOZ_ALWAYS_INLINE_EVEN_DEBUG'
 1:16.06 #  define MOZ_ALWAYS_INLINE_EVEN_DEBUG     __attribute__((always_inline)) inline
 1:16.06                                                                           ^
 1:16.06 In file included from $OBJ/js/src/shell/Unified_cpp_js_src_shell0.cpp:11:
 1:16.06 In file included from $SRC/js/src/shell/js.cpp:13:
 1:16.06 ../../../dist/include/mozilla/mozalloc.h:189:21: error: replacement function 'operator new' cannot be declared 'inline' [-Werror,-Winline-new-delete]
 1:16.06 MOZALLOC_EXPORT_NEW MOZALLOC_INLINE
 1:16.06                     ^
 1:16.06 ../../../dist/include/mozilla/mozalloc.h:31:27: note: expanded from macro 'MOZALLOC_INLINE'
 1:16.06 #  define MOZALLOC_INLINE MOZ_ALWAYS_INLINE_EVEN_DEBUG
 1:16.06                           ^
 1:16.06 ../../../dist/include/mozilla/Attributes.h:27:75: note: expanded from macro 'MOZ_ALWAYS_INLINE_EVEN_DEBUG'
 1:16.06 #  define MOZ_ALWAYS_INLINE_EVEN_DEBUG     __attribute__((always_inline)) inline
 1:16.06                                                                           ^
 1:16.06 In file included from $OBJ/js/src/shell/Unified_cpp_js_src_shell0.cpp:11:
 1:16.06 In file included from $SRC/js/src/shell/js.cpp:13:
 1:16.06 ../../../dist/include/mozilla/mozalloc.h:195:21: error: replacement function 'operator new[]' cannot be declared 'inline' [-Werror,-Winline-new-delete]
 1:16.06 MOZALLOC_EXPORT_NEW MOZALLOC_INLINE
 1:16.06                     ^
 1:16.06 ../../../dist/include/mozilla/mozalloc.h:31:27: note: expanded from macro 'MOZALLOC_INLINE'
 1:16.06 #  define MOZALLOC_INLINE MOZ_ALWAYS_INLINE_EVEN_DEBUG
 1:16.06                           ^
 1:16.06 ../../../dist/include/mozilla/Attributes.h:27:75: note: expanded from macro 'MOZ_ALWAYS_INLINE_EVEN_DEBUG'
 1:16.06 #  define MOZ_ALWAYS_INLINE_EVEN_DEBUG     __attribute__((always_inline)) inline
 1:16.06                                                                           ^
 1:16.06 In file included from $OBJ/js/src/shell/Unified_cpp_js_src_shell0.cpp:11:
 1:16.06 In file included from $SRC/js/src/shell/js.cpp:13:
 1:16.06 ../../../dist/include/mozilla/mozalloc.h:201:21: error: replacement function 'operator new[]' cannot be declared 'inline' [-Werror,-Winline-new-delete]
 1:16.06 MOZALLOC_EXPORT_NEW MOZALLOC_INLINE
 1:16.06                     ^
 1:16.06 ../../../dist/include/mozilla/mozalloc.h:31:27: note: expanded from macro 'MOZALLOC_INLINE'
 1:16.06 #  define MOZALLOC_INLINE MOZ_ALWAYS_INLINE_EVEN_DEBUG
 1:16.06                           ^
 1:16.06 ../../../dist/include/mozilla/Attributes.h:27:75: note: expanded from macro 'MOZ_ALWAYS_INLINE_EVEN_DEBUG'
 1:16.06 #  define MOZ_ALWAYS_INLINE_EVEN_DEBUG     __attribute__((always_inline)) inline
 1:16.06                                                                           ^
 1:16.07 In file included from $OBJ/js/src/shell/Unified_cpp_js_src_shell0.cpp:11:
 1:16.07 In file included from $SRC/js/src/shell/js.cpp:13:
 1:16.07 ../../../dist/include/mozilla/mozalloc.h:207:21: error: replacement function 'operator delete' cannot be declared 'inline' [-Werror,-Winline-new-delete]
 1:16.07 MOZALLOC_EXPORT_NEW MOZALLOC_INLINE
 1:16.07                     ^
 1:16.07 ../../../dist/include/mozilla/mozalloc.h:31:27: note: expanded from macro 'MOZALLOC_INLINE'
 1:16.07 #  define MOZALLOC_INLINE MOZ_ALWAYS_INLINE_EVEN_DEBUG
 1:16.07                           ^
 1:16.07 ../../../dist/include/mozilla/Attributes.h:27:75: note: expanded from macro 'MOZ_ALWAYS_INLINE_EVEN_DEBUG'
 1:16.07 #  define MOZ_ALWAYS_INLINE_EVEN_DEBUG     __attribute__((always_inline)) inline
 1:16.07                                                                           ^
 1:16.07 In file included from $OBJ/js/src/shell/Unified_cpp_js_src_shell0.cpp:11:
 1:16.07 In file included from $SRC/js/src/shell/js.cpp:13:
 1:16.07 ../../../dist/include/mozilla/mozalloc.h:213:21: error: replacement function 'operator delete' cannot be declared 'inline' [-Werror,-Winline-new-delete]
 1:16.07 MOZALLOC_EXPORT_NEW MOZALLOC_INLINE
 1:16.07                     ^
 1:16.07 ../../../dist/include/mozilla/mozalloc.h:31:27: note: expanded from macro 'MOZALLOC_INLINE'
 1:16.07 #  define MOZALLOC_INLINE MOZ_ALWAYS_INLINE_EVEN_DEBUG
 1:16.07                           ^
 1:16.07 ../../../dist/include/mozilla/Attributes.h:27:75: note: expanded from macro 'MOZ_ALWAYS_INLINE_EVEN_DEBUG'
 1:16.07 #  define MOZ_ALWAYS_INLINE_EVEN_DEBUG     __attribute__((always_inline)) inline
 1:16.07                                                                           ^
 1:16.07 In file included from $OBJ/js/src/shell/Unified_cpp_js_src_shell0.cpp:11:
 1:16.07 In file included from $SRC/js/src/shell/js.cpp:13:
 1:16.07 ../../../dist/include/mozilla/mozalloc.h:219:21: error: replacement function 'operator delete[]' cannot be declared 'inline' [-Werror,-Winline-new-delete]
 1:16.07 MOZALLOC_EXPORT_NEW MOZALLOC_INLINE
 1:16.07                     ^
 1:16.07 ../../../dist/include/mozilla/mozalloc.h:31:27: note: expanded from macro 'MOZALLOC_INLINE'
 1:16.07 #  define MOZALLOC_INLINE MOZ_ALWAYS_INLINE_EVEN_DEBUG
 1:16.07                           ^
 1:16.07 ../../../dist/include/mozilla/Attributes.h:27:75: note: expanded from macro 'MOZ_ALWAYS_INLINE_EVEN_DEBUG'
 1:16.07 #  define MOZ_ALWAYS_INLINE_EVEN_DEBUG     __attribute__((always_inline)) inline
 1:16.07                                                                           ^
 1:16.07 In file included from $OBJ/js/src/shell/Unified_cpp_js_src_shell0.cpp:11:
 1:16.07 In file included from $SRC/js/src/shell/js.cpp:13:
 1:16.07 ../../../dist/include/mozilla/mozalloc.h:225:21: error: replacement function 'operator delete[]' cannot be declared 'inline' [-Werror,-Winline-new-delete]
 1:16.07 MOZALLOC_EXPORT_NEW MOZALLOC_INLINE
 1:16.07                     ^
 1:16.07 ../../../dist/include/mozilla/mozalloc.h:31:27: note: expanded from macro 'MOZALLOC_INLINE'
 1:16.07 #  define MOZALLOC_INLINE MOZ_ALWAYS_INLINE_EVEN_DEBUG
 1:16.07                           ^
 1:16.07 ../../../dist/include/mozilla/Attributes.h:27:75: note: expanded from macro 'MOZ_ALWAYS_INLINE_EVEN_DEBUG'
 1:16.07 #  define MOZ_ALWAYS_INLINE_EVEN_DEBUG     __attribute__((always_inline)) inline
 1:16.07                                                                           ^
}


(These don't look really related to js.cpp; I'm unclear on why I'm only getting these warnings now, when I wasn't before. Anyway; let's fix these.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: local hackaround: prevent Winline-new-delete from being treated as errors

Here's the local hackaround that I'm using to allow myself to still build (with warnings-as-errors) -- just telling the compiler not to treat this particular arning as an error.

This isn't a fix -- just a hackaround. I wouldn't want to actually take this on m-c (other than as a band-aid) because this warning is really spammy, as you can see in comment 0. So, we should actually fix it (or completely suppress it, if appropriate).

Comment 2

[Daniel Holbert [:dholbert]]

glandium, do you know what's going on here / can you take a look at this or suggest someone who could?

Comment 3

[Mike Hommey [:glandium]]

See bug 928808.

Comment 4

[Mike Hommey [:glandium]]

(which you should know since you did it ;) )

Comment 5

[Daniel Holbert [:dholbert]]

Psh, that was ages ago. 6-digit bug number. :)

Anyway -- I wonder why that's not helping here. Can't look in depth now, but will later.

(Also, I noticed this, in the MSVC-specific section:
> 2273         if test -n "$CLANG_CL"; then
> 2274             # Suppress the clang-cl warning for the inline 'new' and 'delete' in mozalloc
> 2275             CXXFLAGS="$CXXFLAGS -Wno-inline-new-delete"
> 2276         fi
http://mxr.mozilla.org/mozilla-central/source/configure.in?rev=40957d7011f2#2273

That looks maybe-intended to fix this, but I doubt it has any effect since it's in a MSVC-specific section.

Comment 6

[Mike Hommey [:glandium]]

It doesn't help because js/src/configure.in is not configure.in, and it doesn't have those.

Comment 7

[Daniel Holbert [:dholbert]]

Ah right. So we just need to copy that bug's changes into js/src/configure then.

Comment 9

[(no longer active)]

I have written a patch to do comment 7.

Comment 10

[(no longer active)]

Attachment: Port the -Wno-inline-new-delete option from configure.in to the JS configure script

Comment 11

[(no longer active)]

(In reply to Daniel Holbert [:dholbert] from comment #5)
> Psh, that was ages ago. 6-digit bug number. :)
> 
> Anyway -- I wonder why that's not helping here. Can't look in depth now, but
> will later.
> 
> (Also, I noticed this, in the MSVC-specific section:
> > 2273         if test -n "$CLANG_CL"; then
> > 2274             # Suppress the clang-cl warning for the inline 'new' and 'delete' in mozalloc
> > 2275             CXXFLAGS="$CXXFLAGS -Wno-inline-new-delete"
> > 2276         fi
> http://mxr.mozilla.org/mozilla-central/source/configure.
> in?rev=40957d7011f2#2273
> 
> That looks maybe-intended to fix this, but I doubt it has any effect since
> it's in a MSVC-specific section.

No, this is needed for clang-cl, because it will break in the exact same way as clang without that hunk.

Comment 12

[Daniel Holbert [:dholbert]]

Ah, thanks for explaining the clang-cl chunk. (I'm unfamiliar with clang-cl & its usage w/ MSVC, but I'll take your word for it.)

Thanks for picking this up!

Comment 13

[Daniel Holbert [:dholbert]]

...though, the patch here doesn't apply to current mozilla-inbound.

e.g. this chunk of the patch doesn't apply, because js/src/configure.in doesn't actually have the "unused-local-typedef" contextual line that's quoted in the patch here:

>+++ b/js/src/configure.in
>     # Turn off the following warnings that -Wall turns on:
>     # -Wno-unused - lots of violations in third-party code
>+    # -Wno-inline-new-delete - we inline 'new' and 'delete' in mozalloc
>     # -Wno-unused-local-typedef - catches unused typedefs, which are commonly used in assertion macros
>     #
>     _WARNINGS_CFLAGS="${_WARNINGS_CFLAGS} -Wno-unused"
> 
>+    MOZ_CXX_SUPPORTS_WARNING(-Wno-, inline-new-delete, ac_cxx_has_wno_inline_new_delete)
>     MOZ_CXX_SUPPORTS_WARNING(-Wno-, unused-local-typedef, ac_cxx_has_wno_unused_local_typedef)


MXR link: http://mxr.mozilla.org/mozilla-central/source/js/src/configure.in?rev=7b86dac3ed1b#1244

Comment 14

[(no longer active)]

Sorry, that comes from another patch in my queue!  Please apply manually.  I'll land this properly once glandium reviews the patch.

Comment 15

[Daniel Holbert [:dholbert]]

Comment on attachment 8593939 [details] [diff] [review]
Port the -Wno-inline-new-delete option from configure.in to the JS configure script

As author of bug 928808's patch, I'll r+ this, which I think is safe given the implicit support glandium has expressed here and on bug 928808.

Leaving r?glandium open so he can officially review, but for now I think we should get this in, because it's breaking people's builds -- and then glandium can retroactively r+ [as I'm sure he will] when he has a chance.

Comment 16

[Andrew McCreight [:mccr8]]

Is there some bug on file for getting an OSX build on automation that uses the same version of clang that developers who have up to date versions of XCode on their systems?  It seems bad that we're repeatedly breaking it.

Comment 17

[(no longer active)]

(In reply to Andrew McCreight [:mccr8] from comment #16)
> Is there some bug on file for getting an OSX build on automation that uses
> the same version of clang that developers who have up to date versions of
> XCode on their systems?  It seems bad that we're repeatedly breaking it.

We are using an *ancient* version of clang on our builders.  AFAIK unfortunately nobody owns updating to new compiler versions so there is little point in filing bugs unless someone steps in to help.

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/396382f79f79

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/396382f79f79