Last Comment Bug 701183 - Make MOZ_DELETE use deleted function syntax in compilers other than clang without causing a warning
: Make MOZ_DELETE use deleted function syntax in compilers other than clang wit...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: MFBT (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
Mentors:
Depends on: 700910
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-09 13:44 PST by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-12-17 23:17 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (3.55 KB, patch)
2011-11-10 14:30 PST, Jeff Walden [:Waldo] (remove +bmo to email)
cjones.bugs: review+
Details | Diff | Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-09 13:44:10 PST
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] 2011-11-09 14:52:59 PST
"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] (remove +bmo to email) 2011-11-09 14:58:34 PST
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] 2011-11-09 15:58:58 PST
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] 2011-11-09 16:00:45 PST
(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] (remove +bmo to email) 2011-11-10 14:30:14 PST
Created attachment 573644 [details] [diff] [review]
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] (remove +bmo to email) 2011-11-10 22:30:24 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3a965a12bc8
Comment 7 Marco Bonardo [::mak] 2011-11-11 02:25:42 PST
https://hg.mozilla.org/mozilla-central/rev/c3a965a12bc8

Note You need to log in before you can comment on or make changes to this bug.