Anonymous | Login | 2022-06-25 11:15 UTC | ![]() |
My View | View Issues | Change Log | Roadmap |
View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
0000809 | OMNeT++ | command line tools | public | 2015-02-06 13:29 | 2017-02-13 11:32 | ||||
Reporter | till | ||||||||
Assigned To | rhornig | ||||||||
Priority | low | Severity | minor | Reproducibility | always | ||||
Status | resolved | Resolution | fixed | ||||||
Platform | OS | OS Version | |||||||
Product Version | 4.6 | ||||||||
Target Version | Fixed in Version | 5.1 | |||||||
Summary | 0000809: nedtool generated files generate warnings (msgcppgenerator.cc) | ||||||||
Description | 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 | ||||||||
Steps To Reproduce | 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 | ||||||||
Additional Information | 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 | ||||||||
Tags | No tags attached. | ||||||||
Attached Files | ![]() ![]() ![]() ![]() | ||||||||
![]() |
|
(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. |
![]() |
|||
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 09: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 - 2022 MantisBT Team |