GNOME Bugzilla – Bug 742851
avoid MSVC warnings in G_STMT_END
Last modified: 2015-01-14 15:27:42 UTC
MSVC does not like "while (0)", the attached patch conditionally shuts up the warning in the macro definition. As mentioned in the commit message the trick is borrowed from http://cnicholson.net/2009/03/stupid-c-tricks-dowhile0-and-c4127/ I tested building my project on MSVC 2010 and it works fine.
Created attachment 294423 [details] [review] patch
Created attachment 294424 [details] [review] use G_STMT_START/END in gslice Trivial follow up patch that makes use of the macro instead of open coding do{}while(0)
Created attachment 294425 [details] [review] use G_STMT_START/END in gtestutils Trivial follow up patch that makes use of the macro instead of open coding do{}while(0)
Hi Paolo, It seems like this stems from one building GLib-using code and/or GLib using /W4, which is just below the highest warning level, /Wall (/W4 reports all the warnings sans the ones here[1]), with all possible warnings from Visual Studio turned on. This is why I don't normally see this when building GLib or GLib-using items with Visual Studio. You can take a look at the warning level in the project under "Project Properties", in the C/C++ section, under general. For the Visual Studio project files we have in GNOME, the warning level that is normally set is /W3, which is the normal default-I think it is likely that there will be also other warnings when one uses /W4 (there could be many warnings, multi-platform libraries often hit this, by the way) that we would probably deem to be annoying, but may be useful to catch potential problems in the code during developement. I'm not totally sure whether we really want to do these though, as a result, sorry. p.s. For references, another way to achieve the same effect, would be to update the msvc_recommended_pragmas.h and add something like #pragma warning(disable:4127) and add a comment for that, and make sure that the project force-includes msvc_recommended_pragmas.h to achieve the same effect. This header should be included with every GLib dev files shipped for Windows though. Hope this explains the situation. With blessings! [1]: http://msdn.microsoft.com/en-us/library/23k5d385.aspx
Hi Paolo, ...oh, by the way, for the latter 2 patches, I think they are okay to me if it also works for you on the other platforms as well, the previous comment was solely regarding disabling the C4127 part. With blessings, thank you!
Hi Fan! thanks for the reply. Yes, this is with /W4. I think it is useful to have this patch since this is not only about compiling Glib/Gtk itself, but also for building other projects that use glib.h, for instance I am trying to use /M4 in my own project and this patch already removes a lot of false positives > p.s. For references, another way to achieve the same effect, would be to update > the msvc_recommended_pragmas.h and add something like #pragma > warning(disable:4127) and add a comment for that, and make sure that the > project force-includes msvc_recommended_pragmas.h to achieve the same effect. > This header should be included with every GLib dev files shipped for Windows > though. I do not like this approach: my patch only disable that warning for that specific use of whil(0) not in general for all the sources. If you look on the web it seems a pretty common approach to define multiline macros on MSVC and to be honest I do not see any downside in applying the patch.
Hello Paolo, > ... for instance I am trying to use /M4 in my own project and this patch > already removes a lot of false positives > I do not like this approach: my patch only disable that warning for that > specific use of whil(0) not in general for all the sources. I see what you are getting at... (I thought you wanted to say bye to the C4127 warnings as a whole), then I think it is fine with me when dealing with the other /W4 warnings are fine on your part, as a result. Please go ahead in getting the patches in then. With blessings.
Review of attachment 294423 [details] [review]: Okay after two minor changes. Commit message says nothing about 'G_STMT_END' and it should. ::: glib/gmacros.h @@ +280,3 @@ * This intentionally does not use compiler extensions like GCC's '({...})' to * avoid portability issue or side effects when compiled with different compilers. + * On MSVC we shut up the annoying C4127: “Conditional expression is constant” I'd prefer that this was less confrontationally wordded.
Review of attachment 294424 [details] [review]: Sure. Thanks.
Review of attachment 294425 [details] [review]: Looks fine. The BEGIN { (void) 0; } END thing looks hilariously pointless, but it's not hurting anyone, either...
Reworded both the commit message and the comment and pushed. Thanks!