701183 - Make MOZ_DELETE use deleted function syntax in compilers other than clang without causing a warning

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Jeff Walden [:Waldo]]

Right now MOZ_DELETE expands to |= delete|, for C++11 deleted function syntax, only when compiling with Clang.  GCC and ICC both support deleted function syntax in recent versions, so it should be possible to use the syntax there as well.  The problem is that doing so without compiling with -std=c++0x produces a warning.  It should be possible to work around this warning, or compile with -std=c++0x, to permit MOZ_DELETE to expand to |= delete| with these compilers.

Comment 1

[Nathan Froyd [:froydnj]]

"Working around the warning" requires modifying GCC, at present.  The change is less than five lines of code, if we really wanted to put it in our production compilers.  It's possible upstream would accept some tweaks in this area.

Comment 2

[Jeff Walden [:Waldo]]

I would be astonished if we change our production compilers.  :-)  Whatever happens here is happening primarily for the benefit of developers who don't want to have to sort through warning-spew for every use of MOZ_DELETE.

Changing gcc upstream is feasible, certainly.  gcc's not clang, with still-small uptake, tho, so it's probably not reasonable to just make the fix and assume everyone will get it "soon enough" to not worry about the warning in the interim.

I suspect the only way around this is to resolve the problems preventing us from using -std=c++0x universally, to be honest.  :-\

Comment 3

[Mike Hommey [:glandium]]

The first thing you need to know is: Starting with what GCC version are C++11 deleted functions supported ? Different GCC versions have varying levels of C++0x/C++11 support.

Once you know that, just #if defined(__GXX_EXPERIMENTAL_CXX0X__) && GOOD_VERSION

Comment 4

[Mike Hommey [:glandium]]

(And our build system does use -std=c++0x when it works properly for our use, except under js/src because it was forgotten and now needs some effort to not fail to build with that flag)

Comment 5

[Jeff Walden [:Waldo]]

Attachment: Patch

Ah, didn't realize about __GXX0X_MUMBLE__.  That's convenient.  The gcc mailing lists claim it might eventually be removed in favor of feature-testing on __cplusplus having its C++11 value, so the test is a little more complicated than that, but a comment in the patch explains that.

This patch also:

- adds MOZ_DELETE to all the users that want it in mfbt (which should be a paradigm of good style and thus should use it),
- fixes an inverted comment, noticed by Patrick O'Leary in a comment on my blog post,
- tweaks some comment wording to be clearer about when errors will happen if MOZ_DELETE compiles to nothing, and
- makes code in a comment use the style consistently used throughout mfbt/.

None of those bits are too complicated, hope you don't mind my putting it in this patch.

Comment 6

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3a965a12bc8

Comment 7

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/c3a965a12bc8