View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001969 | OpenFOAM | public | 2016-01-10 00:22 | 2016-01-10 12:51 | |
| Reporter | alexeym | Assigned To | henry | ||
| Priority | normal | Severity | text | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Summary | 0001969: Private methods in protected section of the class (faceZone.H) | ||||
| Description | Another case of private methods being placed in protected section of the class. Since this is not the last report of this type (though is the problem worth reporting?), decided to ask additional questions: 1. According to ISO N4527 (http://open-std.org/JTC1/SC22/WG21/docs/papers/2015/n4527.pdf, pages 290-291), implicitly generated copy constructor has signature X& X::operator=(const X&), yet code puts void X::operator=(const X&) in private section. 2. Since Gcc 4.5 is minimum required version of compiler it is possible to use deleted methods (https://gcc.gnu.org/gcc-4.5/cxx0x_status.html). Is it worth creating patch converting all private copy constructors and assignment operators from private declarations into deleted public (for example in the end of Constructors section of the class)? 3. Also Gcc 4.5 implements correct handling of right brackets in templates, so instead of > > > one can use >>>. Question is the same as in 2, is it worth creating patch converting spaced brackets into dense variant? | ||||
| Steps To Reproduce | 1. Open src/OpenFOAM/meshes/polyMesh/zones/faceZone/faceZone.H 2. Goto line 101 (https://github.com/OpenFOAM/OpenFOAM-dev/blob/master/src/OpenFOAM/meshes/polyMesh/zones/faceZone/faceZone.H#L101) | ||||
| Tags | No tags attached. | ||||
|
|
1. In C++ functions are not resolved based an their return type so it is OK to return void rather than X&. Our preference is to return void from assignment operators as it avoids centain potential programming errors. 2. This would be possible but I am not sure it is worth it at the moment. Once we can drop support for gcc-4 altogether we will be able to move further forward with the use of other more recent C++ stardard functionality. 3. Note that we are supporting other C++ compilers also; is this feature supported by clang (which versions) and the Intel C++ compiler (which versions)? |
|
|
2. OK, since the correction should be added to 804 ("Disallow default ..." comment) + 2 ("Disable default ..." comment) files and I do not yet see the way to automate change, misplaced private methods will appear as bug reports. 3. Yes, clang supports the feature from version 2.9 (http://clang.llvm.org/cxx_status.html), Intel C++ compiler supports it from version 12.0 (https://software.intel.com/en-us/articles/c0x-features-supported-by-intel-c-compiler) |
|
|
2. I do not see any warnings from any version of gcc, clang or icpc I use. How are you generating warnings/errors from this issue? 3. OK, I will look into this change with all the compilers I test with to see if any issues arise. |
|
|
I checked the 2 ("Disable default ..." comment) files and could not see any problems; the disabled functions were in the private section. However, I will make the comments consistent with the rest of the code. > since the correction should be added to 804 ("Disallow default ..." comment) Are you sure this issue occurs in 804 files? How did you check? I think it occurs in very few files. |
|
|
By searching "Disable/Disallow default ..." comment I just estimated number of files where default bitwise constructor/assignment operator could be converted from private to deleted method. Surely not all of them have problem of private/protected misplacement. And since the number of files is 806, conversion is doable but if you think it could wait, let it wait. Unfortunately currently I do not have recipe for automated discovery of misplaced methods (though think certain tool could be developed using libclang), I encounter them randomly. |
|
|
Resolved in OpenFOAM-dev by commit c238fbd56de197bc7cda1df35a1da691e8cd1926 |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-01-10 00:22 | alexeym | New Issue | |
| 2016-01-10 08:50 | henry | Note Added: 0005822 | |
| 2016-01-10 10:00 | alexeym | Note Added: 0005823 | |
| 2016-01-10 10:21 | henry | Note Added: 0005824 | |
| 2016-01-10 10:35 | henry | Note Added: 0005825 | |
| 2016-01-10 11:17 | alexeym | Note Added: 0005826 | |
| 2016-01-10 12:51 | henry | Note Added: 0005827 | |
| 2016-01-10 12:51 | henry | Status | new => resolved |
| 2016-01-10 12:51 | henry | Resolution | open => fixed |
| 2016-01-10 12:51 | henry | Assigned To | => henry |
| 2016-03-11 11:44 | administrator | Category | 3.0.1 => (No Category) |
