Closed
Bug 539694
Opened 15 years ago
Closed 12 years ago
accessible objects should have private copy constructor
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: surkov, Assigned: jhk)
References
(Blocks 1 open bug)
Details
(Keywords: access, Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])
Attachments
(2 files, 2 obsolete files)
907 bytes,
patch
|
Details | Diff | Splinter Review | |
1.43 KB,
patch
|
surkov
:
review+
|
Details | Diff | Splinter Review |
Accessible objects should define copy constructor and assignment operators in private section to avoid accident calls (internal or on AT side). It was inspired by talk with Pete.
Reporter | ||
Comment 1•13 years ago
|
||
check all classes in folder http://mxr.mozilla.org/mozilla-central/source/accessible/src/
Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++]
Comment 2•13 years ago
|
||
it appears you only need to check nsAccessNode and not any of the things that inherit from it because having a private copy constructor / operator = means things inheriting from you can't copy you. Here's a test with gcc 4.6 tbsaunde@football:/tmp$ cat test.cpp class foo { public: foo() { } private: foo& operator =(const foo& aValue); foo(const foo&); }; class dir : public foo { public: dir() : foo() { } int mType; }; int main() { dir bar; dir baz = bar; return baz.mType; } tbsaunde@football:/tmp$ g++ test.cpp test.cpp: In copy constructor ‘dir::dir(const dir&)’: test.cpp:7:3: error: ‘foo::foo(const foo&)’ is private test.cpp:10:7: error: within this context test.cpp: In function ‘int main()’: test.cpp:20:13: note: synthesized method ‘dir::dir(const dir&)’ first required here tbsaunde@football:/tmp$ sorry about the weird characters in the error message, it would appear something between bugzilla and my terminal dislikes utf8 :/
Assignee | ||
Comment 4•13 years ago
|
||
.
Attachment #581202 -
Attachment is obsolete: true
Attachment #581205 -
Flags: review?(trev.saunders)
Attachment #581202 -
Flags: review?(trev.saunders)
Comment 5•13 years ago
|
||
Comment on attachment 581205 [details] [diff] [review] Patch_2 r=tbsaunde not surkov
Attachment #581205 -
Flags: review?(trev.saunders) → review+
Comment 7•13 years ago
|
||
Comment on attachment 581211 [details] [diff] [review] Introduced copy constructor review not needed since same patch.
Attachment #581211 -
Flags: review?(trev.saunders)
Comment 8•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/332cdb521dd6 THANKS!
Comment 9•13 years ago
|
||
actually, https://hg.mozilla.org/integration/mozilla-inbound/rev/54c446091d7c push race :/
Comment 10•13 years ago
|
||
Just to note, there's a new macro you should use to do this, MOZ_DELETE: http://hg.mozilla.org/mozilla-central/annotate/3c321d2c9884/mfbt/Attributes.h#l106 It's just a matter of #include "mozilla/Attributes.h", then putting MOZ_DELETE at the ends of declarations like the ones in comment 9's URL. This has the advantage of producing errors at compile time, not (potentially) at link time (if the code making the accidental copy is a friend of the class being copied).
Comment 11•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/54c446091d7c Leaving open for comment 10.
Assignee: nobody → jigneshhk1992
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla11
Reporter | ||
Comment 12•12 years ago
|
||
Jignesh, could you address comment #10 please?
Assignee | ||
Comment 13•12 years ago
|
||
Attachment #581205 -
Attachment is obsolete: true
Attachment #595446 -
Flags: review?(surkov.alexander)
Reporter | ||
Comment 14•12 years ago
|
||
Comment on attachment 595446 [details] [diff] [review] Thanks for Pointing out. Review of attachment 595446 [details] [diff] [review]: ----------------------------------------------------------------- r=me, thank you for doing this
Attachment #595446 -
Flags: review?(surkov.alexander) → review+
Reporter | ||
Comment 15•12 years ago
|
||
inbound land https://hg.mozilla.org/integration/mozilla-inbound/rev/c6dda929759e
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c6dda929759e
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•