OMNeT++/OMNEST Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0000809OMNeT++command line toolspublic2015-02-06 13:292017-02-13 11:32
Reportertill 
Assigned Torhornig 
PrioritylowSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version4.6 
Target VersionFixed in Version5.1 
Summary0000809: nedtool generated files generate warnings (msgcppgenerator.cc)
DescriptionThe 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
Steps To Reproducecompile 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
Additional InformationI 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
TagsNo tags attached.
Attached Files? file icon msgcppgenerator.cc.patch.3 [^] (10,342 bytes) 2015-02-08 13:38
patch file icon MessageCompilerNowarnings.patch [^] (5,216 bytes) 2017-01-12 16:45 [Show Content]
patch file icon msgcppgenerator.cc.patch [^] (1,024 bytes) 2017-01-12 17:41 [Show Content]
? file icon msgcppgenerator.cc.patch2 [^] (704 bytes) 2017-01-12 18:48

- Relationships

-  Notes
(0000969)
andras (administrator)
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 (administrator)
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 (reporter)
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 (reporter)
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 (reporter)
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 (reporter)
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 (administrator)
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 (reporter)
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 (reporter)
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 (reporter)
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 (administrator)
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 (reporter)
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 (reporter)
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 (reporter)
2017-01-12 17:42

Further patch attached that escapes the @ and adds "pragma clang diagnostic ignored \"-Wreserved-id-macro\"\n""
(0001253)
rhornig (administrator)
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 (reporter)
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 (administrator)
2017-01-12 18:02

Ok, I've merged this too.
(0001256)
rhornig (administrator)
2017-01-12 18:03

Please reopen if you still have issues.
(0001257)
till (reporter)
2017-01-12 18:48

Sorry for the confusion, did you catch the two additional warnings in patch2?
(0001262)
rhornig (administrator)
2017-01-13 14:10

Not yet, but would you mind test it also using GCC? Just to have a complete solution.
(0001263)
rhornig (administrator)
2017-01-13 14:43

Ok, I've added both of them. Also added -Wold-style-cast for the GCC options
(0001265)
rhornig (administrator)
2017-01-16 13:41

All the problematic warnings are now suppressed.
(0001311)
till (reporter)
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 (administrator)
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 (administrator)
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.

- Issue History
Date Modified Username Field Change
2015-02-06 13:29 till New Issue
2015-02-06 13:29 till File Added: msgcppgenerator.cc.patch
2015-02-06 14:27 andras Note Added: 0000969
2015-02-06 14:35 andras Note Added: 0000970
2015-02-06 15:35 till Note Added: 0000971
2015-02-06 15:39 till File Added: msgcppgenerator.cc.patch.2
2015-02-06 15:52 till Note Added: 0000972
2015-02-06 22:16 till Note Added: 0000973
2015-02-08 13:38 till Note Added: 0000974
2015-02-08 13:38 till File Added: msgcppgenerator.cc.patch.3
2015-02-09 11:16 andras Note Added: 0000977
2015-02-09 12:22 till Note Added: 0000981
2015-03-09 09:13 till Note Added: 0000989
2015-10-12 10:28 ammmar1988 Issue cloned: 0000857
2017-01-11 20:37 till Note Added: 0001244
2017-01-12 16:44 rhornig Note Added: 0001248
2017-01-12 16:44 rhornig File Deleted: msgcppgenerator.cc.patch
2017-01-12 16:44 rhornig File Deleted: msgcppgenerator.cc.patch.2
2017-01-12 16:45 rhornig File Added: MessageCompilerNowarnings.patch
2017-01-12 16:48 till Note Added: 0001249
2017-01-12 17:21 till Note Added: 0001251
2017-01-12 17:41 till File Added: msgcppgenerator.cc.patch
2017-01-12 17:42 till Note Added: 0001252
2017-01-12 17:50 rhornig Note Added: 0001253
2017-01-12 17:51 till Note Added: 0001254
2017-01-12 18:02 rhornig Note Added: 0001255
2017-01-12 18:03 rhornig Note Added: 0001256
2017-01-12 18:03 rhornig Status new => resolved
2017-01-12 18:03 rhornig Fixed in Version => 5.1pre3
2017-01-12 18:03 rhornig Resolution open => fixed
2017-01-12 18:03 rhornig Assigned To => rhornig
2017-01-12 18:48 till Note Added: 0001257
2017-01-12 18:48 till Status resolved => feedback
2017-01-12 18:48 till Resolution fixed => reopened
2017-01-12 18:48 till File Added: msgcppgenerator.cc.patch2
2017-01-13 14:10 rhornig Note Added: 0001262
2017-01-13 14:43 rhornig Note Added: 0001263
2017-01-16 13:41 rhornig Note Added: 0001265
2017-01-16 13:41 rhornig Status feedback => resolved
2017-01-16 13:41 rhornig Resolution reopened => fixed
2017-02-08 11:38 till Note Added: 0001311
2017-02-08 11:38 till Status resolved => feedback
2017-02-08 11:38 till Resolution fixed => reopened
2017-02-13 11:18 rhornig Note Added: 0001313
2017-02-13 11:32 rhornig Note Added: 0001314
2017-02-13 11:32 rhornig Status feedback => resolved
2017-02-13 11:32 rhornig Fixed in Version 5.1pre3 => 5.1
2017-02-13 11:32 rhornig Resolution reopened => fixed


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker