Last Comment Bug 711647 - Add MOZ_DELETE to a bunch of deliberately-not-implemented methods
: Add MOZ_DELETE to a bunch of deliberately-not-implemented methods
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: unspecified
: All All
: -- trivial (vote)
: mozilla12
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-16 15:36 PST by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-12-22 03:46 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (26.80 KB, patch)
2011-12-16 15:36 PST, Jeff Walden [:Waldo] (remove +bmo to email)
dbaron: review+
Details | Diff | Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-16 15:36:57 PST
Created attachment 582414 [details] [diff] [review]
Patch

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
{
  private:
    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: <http://trac.webkit.org/changeset/68414>.  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.)
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-12-20 08:32:56 PST
Comment on attachment 582414 [details] [diff] [review]
Patch

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
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-21 19:05:01 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c7cc49f6556

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.
Comment 3 Ed Morley [:emorley] 2011-12-22 03:46:42 PST
https://hg.mozilla.org/mozilla-central/rev/9c7cc49f6556

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