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 761119 - GCC 6 --pedantic: enumerator value for ‘G_PARAM_DEPRECATED’ is not an integer constant expression
GCC 6 --pedantic: enumerator value for ‘G_PARAM_DEPRECATED’ is not an integer...
Status: RESOLVED DUPLICATE of bug 767882
Product: glib
Classification: Platform
Component: gobject
2.50.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2016-01-26 07:35 UTC by Octavio Alvarez Piza
Modified: 2016-10-24 15:49 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Octavio Alvarez Piza 2016-01-26 07:35:35 UTC
Programs compiled using --pedantic-errors fail to compile under GCC 6 if using GObject (directly or indirectly). Please see the following log:

[  0][Mon Jan 25 23:21:15 -0800 -- alvarezp@alvarezp-samsung:~/temp/glib-test]
$ gcc-6 --pedantic-errors $(pkg-config glib-2.0 --cflags) -c glib-test.c
In file included from /usr/include/glib-2.0/gobject/gobject.h:26:0,
                 from /usr/include/glib-2.0/gobject/gbinding.h:29,
                 from /usr/include/glib-2.0/glib-object.h:23,
                 from glib-test.c:1:
/usr/include/glib-2.0/gobject/gparam.h:166:33: error: enumerator value for ‘G_PARAM_DEPRECATED’ is not an integer constant expression [-Wpedantic]
   G_PARAM_DEPRECATED          = 1 << 31
                                 ^


[  1][Mon Jan 25 23:23:30 -0800 -- alvarezp@alvarezp-samsung:~/temp/glib-test]
$ !cat
cat glib-test.c
#include <glib-object.h>

int main(void) {
	return 0;
}

The problem seems to be that 1 << 31 is not a valid value because it can't fit in a signed integer.
Comment 1 Octavio Alvarez Piza 2016-01-26 07:42:47 UTC
This is the relevant portion of gparam.h:

$ cat -n /usr/include/glib-2.0/gobject/gparam.h | grep -C 5 '^ *166'
   161	#endif
   162	  G_PARAM_STATIC_NICK	      = 1 << 6,
   163	  G_PARAM_STATIC_BLURB	      = 1 << 7,
   164	  /* User defined flags go here */
   165	  G_PARAM_EXPLICIT_NOTIFY     = 1 << 30,
   166	  G_PARAM_DEPRECATED          = 1 << 31
   167	} GParamFlags;
   168	
   169	/**
   170	 * G_PARAM_STATIC_STRINGS:
   171	 * 

Would a possible fix be to change G_PARAM_DEPRECATED to 1 << 29?
Comment 2 Dan Winship 2016-01-26 13:11:09 UTC
No, changing the value would be an API break.

Does

  G_PARAM_DEPRECATED          = 0x80000000 /* 1 << 31 */

work without warnings? If not, what about "-0x80000000"?
Comment 3 Octavio Alvarez Piza 2016-01-26 17:47:35 UTC
Neither of those worked:

[  0][Mon Jan 25 23:39:58 -0800 -- alvarezp@alvarezp-samsung:~/temp/glib-test]
$ gcc-6 --pedantic-errors $(pkg-config glib-2.0 --cflags) -c glib-test.c
In file included from /usr/include/glib-2.0/gobject/gobject.h:26:0,
                 from /usr/include/glib-2.0/gobject/gbinding.h:29,
                 from /usr/include/glib-2.0/glib-object.h:23,
                 from glib-test.c:1:
/usr/include/glib-2.0/gobject/gparam.h:166:33: error: ISO C restricts enumerator values to range of ‘int’ [-Wpedantic]
   G_PARAM_DEPRECATED          = 0x80000000
                                 ^~~~~~~~~~


[  1][Tue Jan 26 09:43:40 -0800 -- alvarezp@alvarezp-samsung:~/temp/glib-test]
$ gcc-6 --pedantic-errors $(pkg-config glib-2.0 --cflags) -c glib-test.c
In file included from /usr/include/glib-2.0/gobject/gobject.h:26:0,
                 from /usr/include/glib-2.0/gobject/gbinding.h:29,
                 from /usr/include/glib-2.0/glib-object.h:23,
                 from glib-test.c:1:
/usr/include/glib-2.0/gobject/gparam.h:166:33: error: ISO C restricts enumerator values to range of ‘int’ [-Wpedantic]
   G_PARAM_DEPRECATED          = -0x80000000
                                 ^

[  1][Tue Jan 26 09:44:01 -0800 -- alvarezp@alvarezp-samsung:~/temp/glib-test]
$ cat glib-test.c
#include <glib-object.h>

int main(void) {
	return 0;
}
Comment 4 Emmanuele Bassi (:ebassi) 2016-01-26 17:53:15 UTC
My usual answer for people using `-Wpedantic` and experiencing issues is: stop using `-Wpedantic`. It buys you nothing, and makes your (and everybody else's) life miserable.
Comment 5 Octavio Alvarez Piza 2016-01-27 04:59:20 UTC
The problem is not -Wpedantic, it just helped detect it and it serves the purpose of let the issue show up and having a potentially automated criteria for regression testing.
Comment 6 Matthias Clasen 2016-01-27 13:41:00 UTC
I don't think 'builds with pedantic' should be part of regression test criteria
Comment 7 Emmanuele Bassi (:ebassi) 2016-01-27 13:54:20 UTC
(In reply to Octavio Alvarez Piza from comment #5)
> The problem is not -Wpedantic, it just helped detect it and it serves the
> purpose of let the issue show up and having a potentially automated criteria
> for regression testing.

The issue is that, while the ISO C99 spec says that enumeration types have to be big enough to fit into a signed integer, the reality is that the spec leaves the *maximum* storage size for an integer (and how to determine it) wholly undefined and up to the implementation.

GCC and Clang (and MSVC) are fine with expanding the storage for enums to unsigned integers and, in case of a bigger constant, to signed/unsigned longs; we haven't had any issue reported with other compilers conforming to our own toolchain requirements: https://wiki.gnome.org/Projects/GLib/CompilerRequirements

That definition, in particular, has been introduced in 2010 and it has survived multiple releases of GLib on multiple platforms.
Comment 8 Matthias Clasen 2016-01-27 15:50:35 UTC
I've added something about 'big enums' to that page now.
Comment 9 Octavio Alvarez Piza 2016-01-27 19:21:48 UTC
This silences the warning:

  G_PARAM_DEPRECATED          = (int) (1U << 31)

Is this acceptable? Is it reliable?
Comment 10 Octavio Alvarez Piza 2016-02-02 19:35:40 UTC
Is there any information I can offer or any test I can do to continue with the resolution of this bug?
Comment 11 Michael Catanzaro 2016-03-01 00:27:12 UTC
(In reply to Octavio Alvarez Piza from comment #10)
> Is there any information I can offer or any test I can do to continue with
> the resolution of this bug?

I'm gonna suggest using -isystem to include header files outside your project, so you don't get warnings that you have no control over to fix. That's really easy to do with CMake, you just add SYSTEM to your include_directories() command. But TBH I don't know how to do it with Autotools, it's not conventional there.
Comment 12 Michael Catanzaro 2016-03-01 00:29:24 UTC
Closing due to Mathias's statement on portability: "Some of our enum types use 1<<31 as a value. We also use negative values in enums. We rely on the compiler to choose a suitable storage size for the enum that can accommodate this."
Comment 13 Daniel Boles 2016-10-23 23:12:02 UTC
Given this piece of Standardese:

> Otherwise, if E1 has a signed type and non-negative value,
> and E1×2E^2 is representable in the result type,
> then that is the resulting value;
> otherwise, the behavior is undefined.

By my parse of this, shifting the non-negative 1 into/past the sign bit invokes integer overflow and hence UB. It's weird that GCC doesn't just warn about that rather more obvious issue, instead of throwing the 'not a constant expression' red herring - but hey, that's nothing new.

This is entrenched in code now - assuming all/most callers' compilers managed to deduce the same value through the apparent UB. The fix is probably to replace the 1 << 31 with an alternative expression, which holds the value that you intended, and which (most) callers' compilers evaluate this to.

I don't think this is an issue really of bit width, but rather of the signed underlying type and the undefined behaviour caused by overflowing it, and/or making assumptions about its sign representation
Comment 14 Daniel Boles 2016-10-24 13:13:17 UTC
for ref, a fix for this has been pushed from https://bugzilla.gnome.org/show_bug.cgi?id=767882
Comment 15 Daniel Boles 2016-10-24 13:17:28 UTC

*** This bug has been marked as a duplicate of bug 767882 ***
Comment 16 Octavio Alvarez Piza 2016-10-24 15:49:27 UTC
Thank you very much not only for fixing the bug but for fixing the negligence with which this bug was handled.

--pedantic has its uses. The fact that developers disdain it like this says a lot about the developer attitude towards code quality. Tools are not perfect, but we need them to improve the quality of the whole ecosystem.

Furthermore, when developers of such a library deliberately neglect their users the use of tools like this, it results in harm to the whole community.