View Issue Details

IDProjectCategoryView StatusLast Update
0001969OpenFOAM(No Category)public2016-01-10 12:51
ReporteralexeymAssigned Tohenry 
PrioritynormalSeveritytextReproducibilityalways
Status resolvedResolutionfixed 
Product Version 
Fixed in Version 
Summary0001969: Private methods in protected section of the class (faceZone.H)
DescriptionAnother 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 Reproduce1. 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)
TagsNo tags attached.

Activities

henry

2016-01-10 08:50

manager   ~0005822

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)?

alexeym

2016-01-10 10:00

reporter   ~0005823

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)

henry

2016-01-10 10:21

manager   ~0005824

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.

henry

2016-01-10 10:35

manager   ~0005825

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.

alexeym

2016-01-10 11:17

reporter   ~0005826

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.

henry

2016-01-10 12:51

manager   ~0005827

Resolved in OpenFOAM-dev by commit c238fbd56de197bc7cda1df35a1da691e8cd1926

Issue History

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)