A few code questions
During ongoing attempt to build OpenFOAM with "-Wall -Wextra -std=c++11" flags without any warnings the following questions arose. Since they are not bugs and not a feature proposal yet, I have decided to discuss it here.
1. C++11 standard deprecates register variable modifier. Yet it quite frequently used in base library. Should it be kept in the code? 2. There are plenty of unused parameters in the code, which way of suppressing warning is preferred? There are several common methods: use UNUSED macro (which is basically UNUSED(x) (void)x), comment-out name of the parameter, just remove parameter name. 3. Though both g++ and clang produce code that can deal with NULL-pointer dereferencing into constant reference (directAddressing in FieldMapper.H is an example), stiil this behavior is undefined in standard. Should these parts kept as they are or should pointer-returning methods added to those cases? 4. In most cases implementation of child classes is OK, but from time to time developers desire strange things (functionObjects library is an example) like return type overload. I.e. functionObject class is a child of regIOobject, yet it is not happy with write method that returns bool, it would like to have write method returning nothing (i.e. void). Should such cases be prohibited (i.e. enforce correct method overload) or left as they are (allow method hiding)? 5. This question is just about author intentions in readKivaGrid.H file. There is a loop on line 426 Code:
for Code:
for 6. This question is about author intention in surfaceBooleanFeatures.C. On line 357 there is calcFeaturePoints function. At the end of this function the following variables are defined: Code:
label concaveStart = 0; |
Greetings Alexey,
I won't be able to answer all of your questions, but let me see what I can answer: Quote:
Quote:
Preferably, the ".H" files that result in these warnings should be identified and separated into an hierarchical structure, e.g. (fake names used here):
Quote:
Code:
virtual const labelListList * addressing() const Quote:
Code:
bool result = object.write(); Nothing worst than ambiguity in coding... specially if one compiler chooses one method over the other on its own. So I guess that yes, the overloads should be stricter and if there is a reason for not being so strict, then the name of the methods should be different, to avoid collision or ambiguity. Quote:
For example: Code:
- for Worst even, since it's an iterator, it might loop around on its own... so it might make sense to only stop for the second iterator... Nah, something doesn't add up. This loop needs to be revised properly. This simply feels like accidentally bad coding went on in this loop. If the lists are not of the same size... OK, it's a wedge, so it's by definition always of the same size... or at least it should always be. It seems that this code was left as-is for so long, because of the following premise:
As for the origin of this bug... it seems to exist at least since OpenFOAM 1.1. Quote:
Beyond this, I suggest that you deliver each patch individually (roughly 1 per question), whenever possible, otherwise it'll be a lot more complicated for the changes to be accepted. For discussion, it might make sense to report each fix in independent bug reports ;) Best regards, Bruno |
Hi Bruno,
Thanks for the answer. Quote:
Quote:
Code:
template<class T, class BinaryOp> 1. Comment out parameter names Code:
template<class T, class BinaryOp> Code:
template<class T, class BinaryOp> Code:
template<class T, class BinaryOp> Quote:
Code:
virtual const labelUList& directAddressing() const Code:
if (&map.directAddressing()) Quote:
1. http://www.open-std.org/jtc1/sc22/wg...2011/n3242.pdf, 7.1.1 2. http://www.drdobbs.com/keywords-that...noth/184403859 3. http://code.qt.io/cgit/qt/qt.git/tre...31653a71#n1504 |
Hi Alexey,
Sorry for the late reply... Quote:
I guess it depends on what is the oldest GCC version the OpenFOAM Foundation is willing to support for the latest OpenFOAM versions. They dropped support for GCC 4.4.x sometime ago in what seemed something mostly related to "we don't have time to test with so many GCC versions" than anything else. Quote:
The "Q_UNUSED" solution seems like an unnecessary amount of code and resulting binary code. Although... I very vaguely remember seeing in OpenFOAM either solutions #1 or #2... but I can't remember where exactly and which ones :( Wait... after a quick search... oh the irony... the names are omitted when issuing the FatalError message itself, but the method has the variables identified, e.g.: Code:
Foam::label Foam::ptscotchDecomp::decompose Quote:
Your proposition seems to make sense... problem is: how would that affect performance? Best regards, Bruno |
Dear Bruno,
Thanks for the answer. Quote:
Quote:
Quote:
|
All times are GMT -4. The time now is 02:04. |