After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 753310 - Remove `#pragma GCC system_header` from gmessages.h
Remove `#pragma GCC system_header` from gmessages.h
Product: glib
Classification: Platform
Component: general
Other Linux
: Normal minor
: ---
Assigned To: gtkdev
Depends on:
Reported: 2015-08-06 08:50 UTC by Milan Crha
Modified: 2015-09-22 14:16 UTC
See Also:
GNOME target: ---
GNOME version: ---

Revert use of the system_header GCC pragma (1.51 KB, patch)
2015-09-22 12:12 UTC, Emmanuele Bassi (:ebassi)
committed Details | Review

Description Milan Crha 2015-08-06 08:50:24 UTC
The gmessages.h contains a 
   #pragma GCC system_header
pragma, which hides compiler warnings when macros like g_return_if_fail() is used in a function which should return value (or vice versa), or when an incorrect value type is returned in the macro. Such compiler warnings help to identify dumb coding issues.

This change had been added in commit 662bf991, at the end of 2001. I believe it's time to revert the commit and let the compiler do its job.
Comment 1 Milan Crha 2015-08-06 08:52:12 UTC
Here's a downstream bug report where can be found a test program too.
Comment 2 Michael Natterer 2015-09-21 18:58:14 UTC
2001? GCC did warn about this just fine until recently.
Comment 3 Emmanuele Bassi (:ebassi) 2015-09-22 12:00:17 UTC
(In reply to Michael Natterer from comment #2)
> 2001? GCC did warn about this just fine until recently.

It used to, but stopped doing that since GCC 4.8 or 4.9.

Clang still warns correctly. I think the commit in question should be reverted, otherwise we're missing valuable warning messages.
Comment 4 Emmanuele Bassi (:ebassi) 2015-09-22 12:12:56 UTC
Created attachment 311851 [details] [review]
Revert use of the system_header GCC pragma

This reverts commit 662bf991c08b16dea8a36026243b311f6cdb17f1. It is not
a straight up revert because the old commit involved various long since
removed ChangeLog files and we'd end up mudding the patch.

The system_header GCC pragma is breaking warnings in the various
g_return_* macros; GCC stopped warning when using a macro with a return
value in a function that returns void, as well as when using a macro
with no return value in a function that has a non-void return value.
Suppressing this kind of warnings is not a good idea.

Other compilers are unaffected, even ones like Clang with a GCC
compatibility layer.

Given the fact that the original commit was added 14 years ago as a
workaround in the old days of GTK+ 1.2, I think it's safe to drop it.
Comment 5 Colin Walters 2015-09-22 14:07:42 UTC
Review of attachment 311851 [details] [review]:

I suspect this is going to have some fallout for people who use -Werror, but they signed up for pain.
Comment 6 Emmanuele Bassi (:ebassi) 2015-09-22 14:16:12 UTC
Thanks for the review, Colin.

The warnings have always been there, so I hope nobody using -Werror is relying on GLib/GCC not warning any more.

Also, auto-builders and jhbuild users should be shielded by gnome-common/m4-common. At least, I hope.

Attachment 311851 [details] pushed as ab26dd5 - Revert use of the system_header GCC pragma