OMNeT++/OMNEST Bug Tracker - OMNeT++
View Issue Details
0000809OMNeT++command line toolspublic2015-02-06 13:292017-02-13 11:32
till 
rhornig 
lowminoralways
resolvedfixed 
4.6 
5.1 
0000809: nedtool generated files generate warnings (msgcppgenerator.cc)
The classes generated by nedtool are not perfectly clean. We love to enable more warnings and also compile with -Werror, but that is not possible due to the sometimes not so clean code that was generated.

The warnings found are mostly related to:
- unreachable code
- old style casts
- missing casts
- unused attributes
compile with these added to CFLAGS:
Wall -Wunused -Wextra -Winit-self -Wswitch-enum -Wuninitialized -Wfloat-equal -Wsuggest-attribute=pure -Wsuggest-attribute=const -Wsuggest-attribute=noreturn -Wsuggest-attribute=format -Wconversion -Wuseless-cast -Wshadow -Werror
I attached a patch to resolve the problems, but it should be carefully reviewed and tested!

I have resolved some warnings, sometimes it is only possible to disable warnings for the particular part of code using pragma (e.g. because cast depends on 32/64 Bit or because it is a suggestion (attribute=noreturn))

Also I was wondering about line 1594:
    CC << " " << info.msgclass << " *pp = (" << info.msgclass << " *)object; (void)pp;\n";

The "(void)pp;" does nothing! right?


I hope you can use the patch. :)
Best regards
Till
No tags attached.
? msgcppgenerator.cc.patch.3 (10,342) 2015-02-08 13:38
https://dev.omnetpp.org/bugs/file_download.php?file_id=155&type=bug
patch MessageCompilerNowarnings.patch (5,216) 2017-01-12 16:45
https://dev.omnetpp.org/bugs/file_download.php?file_id=202&type=bug
patch msgcppgenerator.cc.patch (1,024) 2017-01-12 17:41
https://dev.omnetpp.org/bugs/file_download.php?file_id=203&type=bug
? msgcppgenerator.cc.patch2 (704) 2017-01-12 18:48
https://dev.omnetpp.org/bugs/file_download.php?file_id=204&type=bug
Issue History
2015-02-06 13:29tillNew Issue
2015-02-06 13:29tillFile Added: msgcppgenerator.cc.patch
2015-02-06 14:27andrasNote Added: 0000969
2015-02-06 14:35andrasNote Added: 0000970
2015-02-06 15:35tillNote Added: 0000971
2015-02-06 15:39tillFile Added: msgcppgenerator.cc.patch.2
2015-02-06 15:52tillNote Added: 0000972
2015-02-06 22:16tillNote Added: 0000973
2015-02-08 13:38tillNote Added: 0000974
2015-02-08 13:38tillFile Added: msgcppgenerator.cc.patch.3
2015-02-09 11:16andrasNote Added: 0000977
2015-02-09 12:22tillNote Added: 0000981
2015-03-09 09:13tillNote Added: 0000989
2015-10-12 09:28ammmar1988Issue cloned: 0000857
2017-01-11 20:37tillNote Added: 0001244
2017-01-12 16:44rhornigNote Added: 0001248
2017-01-12 16:44rhornigFile Deleted: msgcppgenerator.cc.patch
2017-01-12 16:44rhornigFile Deleted: msgcppgenerator.cc.patch.2
2017-01-12 16:45rhornigFile Added: MessageCompilerNowarnings.patch
2017-01-12 16:48tillNote Added: 0001249
2017-01-12 17:21tillNote Added: 0001251
2017-01-12 17:41tillFile Added: msgcppgenerator.cc.patch
2017-01-12 17:42tillNote Added: 0001252
2017-01-12 17:50rhornigNote Added: 0001253
2017-01-12 17:51tillNote Added: 0001254
2017-01-12 18:02rhornigNote Added: 0001255
2017-01-12 18:03rhornigNote Added: 0001256
2017-01-12 18:03rhornigStatusnew => resolved
2017-01-12 18:03rhornigFixed in Version => 5.1pre3
2017-01-12 18:03rhornigResolutionopen => fixed
2017-01-12 18:03rhornigAssigned To => rhornig
2017-01-12 18:48tillNote Added: 0001257
2017-01-12 18:48tillStatusresolved => feedback
2017-01-12 18:48tillResolutionfixed => reopened
2017-01-12 18:48tillFile Added: msgcppgenerator.cc.patch2
2017-01-13 14:10rhornigNote Added: 0001262
2017-01-13 14:43rhornigNote Added: 0001263
2017-01-16 13:41rhornigNote Added: 0001265
2017-01-16 13:41rhornigStatusfeedback => resolved
2017-01-16 13:41rhornigResolutionreopened => fixed
2017-02-08 11:38tillNote Added: 0001311
2017-02-08 11:38tillStatusresolved => feedback
2017-02-08 11:38tillResolutionfixed => reopened
2017-02-13 11:18rhornigNote Added: 0001313
2017-02-13 11:32rhornigNote Added: 0001314
2017-02-13 11:32rhornigStatusfeedback => resolved
2017-02-13 11:32rhornigFixed in Version5.1pre3 => 5.1
2017-02-13 11:32rhornigResolutionreopened => fixed

Notes
(0000969)
andras   
2015-02-06 14:27   
"(void)pp;" is a trick to suppress the "unused variable" warning in a compiler-independent way. It's commonly used in generated code as it's way simpler than littering the generator with lots of conditionals.

See e.g. http://stackoverflow.com/questions/1486904/how-do-i-best-silence-a-warning-about-unused-variables [^]
(0000970)
andras   
2015-02-06 14:35   
The problem with __attribute__((unused)) and #pragma GCC is that they are GCC specific. We'd like OMNeT++ to compile with as many C++ compilers as possible. Even pragmas generate "unrecognized pragma" warnings with many compilers.

Do you want me to wait for an updated patch, or use parts of the current one?
(0000971)
till   
2015-02-06 15:35   
I could wrap the #pragma in #ifdefs to be sure that there is a gcc. Clang does not know the useless-cast warning (as far as I know). Are you sure that __attribute__((unused)) causes problems in other compilers?

About the "(void)pp;"
Would be better to have the pp variable only when it is necessary, but that would blow up the code right? We can leave that there...

I will upload a new patch in a moment. Just take what you think is safe. The __attribute__((unused)) that I added could also be replaced by the (void*) hack :)
(0000972)
till   
2015-02-06 15:52   
Oh, I just saw that I missed some other old style cast!

I come up with another version :(
(0000973)
till   
2015-02-06 22:16   
I found a few more issues when trying to compile omnet++ with my changes :(

any idea how to cast from "const type" or "const type *" to "void *" without using an old style cast?
(0000974)
till   
2015-02-08 13:38   
Patch 3 seems to be working but it still has one old-sytle cast in ::getFieldStructPointer(void *object, int field, int i) const that I cannot get rid of. Any hint would be appreciated!
(0000977)
andras   
2015-02-09 11:16   
Thanks, I'll check. It's a little more work because in the upcoming 5.0 branch the code generator looks a bit different (due to some cClassDescriptor changes, etc.) BTW we're planning a first beta of 5.0 "real soon" (next week?)
(0000981)
till   
2015-02-09 12:22   
Hi Andras,

Yes, If you find a way to remove the last old-style cast that would be great.

I'm really happy to hear that 5.0 is coming I will try it out as soon as I get my hands on it! Any chance to see Ned-Inheritance? :) http://dev.omnetpp.org/bugs/view.php?id=756 [^]
(0000989)
till   
2015-03-09 09:13   
follow up:

"any idea how to cast from "const type" or "const type *" to "void *" without using an old style cast?"

This does only work with two casts. A const_cast has to be done first!
(0001244)
till   
2017-01-11 20:37   
can I bring the warnings in _m.h and _m.cc files up again for the 5.1 release? It would be so great if the Problems View would not be polluted by these hundreds of warnings.
(0001248)
rhornig   
2017-01-12 16:44   
expected warnings were specifically disabled in the header and _m.cc file for both GCC and CLANG. On GCC now builds with

-Wall -Wpedantic -Wunused -Wextra -Winit-self -Wswitch-enum -Wuninitialized -Wfloat-equal -Wsuggest-attribute=pure -Wsuggest-attribute=const -Wsuggest-attribute=noreturn -Wsuggest-attribute=format -Wconversion -Wuseless-cast -Wshadow
    
without generating any warnings (tested with INET message files).

Additionally omnetpp.h was declared a "system header" so all warnings coming from there are muted.

Please see the attached pacth file
(0001249)
till   
2017-01-12 16:48   
Thank you Rudolf, I will give the patch a try and let you know how it looks like in my installation!
(0001251)
till   
2017-01-12 17:21   
Ok, just tested it. I only have two left: -Wreserved-id-macro due to forbidden __ in defines and -Wdocumentation-unknown-command due to unescaped @


e.g.:
./core4inet/scheduler/SchedulerMessage_m.h:6:9: warning: macro name is a reserved identifier [-Wreserved-id-macro]
#define __CORE4INET_SCHEDULERMESSAGE_M_H
        ^
./core4inet/scheduler/SchedulerMessage_m.h:72:8: warning: unknown command tag name [-Wdocumentation-unknown-command]
 * @customize(true);
       ^
(0001252)
till   
2017-01-12 17:42   
Further patch attached that escapes the @ and adds "pragma clang diagnostic ignored \"-Wreserved-id-macro\"\n""
(0001253)
rhornig   
2017-01-12 17:50   
Thanks I will check/add this. Sadly we are using __ prefixed header guards everywhere in OMNETPP and INET :(. It was not the best choice in this regards. Is the reserved-id-macro just a problem with clang? GCC does not care about this?
(0001254)
till   
2017-01-12 17:51   
Ahrg no wait a moment! for some reason I missed more warnings, will come up with another patch!:
-Wold-style-cast
-Wunreachable-code-break
(0001255)
rhornig   
2017-01-12 18:02   
Ok, I've merged this too.
(0001256)
rhornig   
2017-01-12 18:03   
Please reopen if you still have issues.
(0001257)
till   
2017-01-12 18:48   
Sorry for the confusion, did you catch the two additional warnings in patch2?
(0001262)
rhornig   
2017-01-13 14:10   
Not yet, but would you mind test it also using GCC? Just to have a complete solution.
(0001263)
rhornig   
2017-01-13 14:43   
Ok, I've added both of them. Also added -Wold-style-cast for the GCC options
(0001265)
rhornig   
2017-01-16 13:41   
All the problematic warnings are now suppressed.
(0001311)
till   
2017-02-08 11:38   
Unfortunately I got another one:
We further have to exclude: -Winconsistent-missing-override

coming from:

warning: 'getDisplayString' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
    virtual const char * getDisplayString() const;
(0001313)
rhornig   
2017-02-13 11:18   
This warning seems to be useful and I would not want to suppress it globally. On the other hand it's not possible to specify in the message file whether the methods should be defined as 'override'. Mostly because the message compiler does not know anything about the base class of a message if it is in a different file. Can't you somehow insert the necessary pragmas into the C++ code to suppress the warning in this special case? (i.e. when you indeed intend to override getDisplayString?

Or an other approach would be to handle the name "displayString" as a special case?
(0001314)
rhornig   
2017-02-13 11:32   
After a bit more thinking I recommend adding the necessary pragma options in the cplusplus {{ }} section inside the message file to workaround this particular issue.