[SERVER-10664] template ptr<T> class not const correct Created: 02/Sep/13 Updated: 10/Dec/14 Resolved: 09/Sep/13 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | 2.5.2 |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Michael H. Cox | Assignee: | Andy Schwerin |
| Resolution: | Done | Votes: | 0 |
| Labels: | pull-request | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
C++ generic |
||
| Participants: |
| Description |
|
I was browsing the mongodb code in doxygen (I'm thinking of "stealing" your ptr<T> class) and noticed that the operator->() and operator*() (dereference) are not const correct: T* operator->() const { return _p; }T& operator*() const { return *_p; } operator T* () const { return _p; } should be: const T* operator->() const { return _p; }const T& operator*() const { return *_p; } operator const T* () const { return _p; } // ... and if you want non-const versions. T& operator*() { return *_p; } operator T* () { return _p; } If there is some rationale for violating the const correctness, it should at least be documented in a comment (didn't notice anything in the doxygen documentation regarding this). |
| Comments |
| Comment by Andy Schwerin [ 09/Sep/13 ] |
|
mhcox, no sweat. Thanks for doing the due diligence. |
| Comment by Michael H. Cox [ 05/Sep/13 ] |
|
After reviewing how C++11 specifies shared_ptr<T> (20.7.2.2.5) and unique_ptr<T> (20.7.1.2.4) operator->() and operator*(), ptr<T> is using the same design pattern and therefore is correct. Sorry about wasting your time. At least I learned how to run the build and unit-tests as well as creating a pull request. :-} |
| Comment by Michael H. Cox [ 03/Sep/13 ] |
|
Also ran "scons smokeCppUnittests" with all tests passing. |
| Comment by Michael H. Cox [ 02/Sep/13 ] |
|
Also ran "scons all" and "scons test". Appeared to work successfully. Not absolutely sure (build/moduletests.txt was empty). |
| Comment by Michael H. Cox [ 02/Sep/13 ] |
|
Corrected const incorrectness of ptr<T> class in pull request from github.com/mhcox/mongo on branch mhcox/ |