Closed Bug 711647 Opened 10 years ago Closed 10 years ago

Add MOZ_DELETE to a bunch of deliberately-not-implemented methods


(Core :: General, defect)

Not set





(Reporter: Waldo, Assigned: Waldo)



(1 file)

Attached patch PatchSplinter Review
On a lark, early this morning when I couldn't fall back asleep before the time I planned to wake up, I did some really dumb MXR searches for deliberately-unimplemented operator=(T) implementations and fixed a smattering of them.  There's no complexity here (note the nsString.h change pairs to the nsTString.h change), but 1) there are enough changes across code I have no relation to that someone really should look, and 2) does whatever SDK we have include all headers in EXPORTS (the Attributes.h header is exported that way) now, such that the additions to the string headers will work correctly for SDK builds?  (It all builds fine locally, but I don't know the incantation to package an SDK.)

Incidentally, if you had any thoughts that inheriting from a non-copyable class might be preferable for many of these uses, like so:

class Noncopyable
    Noncopyable(const Noncopyable&) MOZ_DELETE;
    void operator=(const Noncopyable&) MOZ_DELETE;

class nsCSSParser : private Noncopyable

It turns out this pattern has adverse impact on the inheritor's sizeof with some ABIs, in at least some circumstances: <>.  So the way to do this is with deleted declarations in the actual class, however that happens.  (I prefer the explicitness of seeing the deleted declarations to a MOZ_DISALLOW_COPYING(nsCSSParser) macro, myself.)
Attachment #582414 - Flags: review?(dbaron)
Comment on attachment 582414 [details] [diff] [review]

I thought some of the removed comments would have been nice to keep, in
particular, the ones in gfxQuartsNativeDrawing.h, jscompartment.h,
Debugger.h, mozJSComponentLoader.h, xpcprivate.h,·

nsZipArchive.h:  leave the "private methods" comment where it was

DeadlockDetector.h: should be in private: rather than public:

r=dbaron with that
Attachment #582414 - Flags: review?(dbaron) → review+

I didn't change the JS instances, because in the past we've had discussions of the private-copy/assignment ops pattern and decided it was idiomatic and did not need comments.  I changed the rest of them, tho.
Target Milestone: --- → mozilla12
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.