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

RESOLVED FIXED in mozilla11



8 years ago
7 years ago


(Reporter: Waldo, Assigned: Waldo)



Firefox Tracking Flags

(Not tracked)



(1 attachment)



8 years ago
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.
"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

8 years ago
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.  :-\
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
(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

8 years ago
Posted patch PatchSplinter Review
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.
Assignee: nobody → jwalden+bmo
Attachment #573644 - Flags: review?(jones.chris.g)
Attachment #573644 - Flags: review?(jones.chris.g) → review+
Last Resolved: 8 years ago
Resolution: --- → FIXED


7 years ago
Component: General → MFBT
QA Contact: general → mfbt
You need to log in before you can comment on or make changes to this bug.