This might be harder than it seems -- G_STATIC_ASSERT is in a public header which means it may end up being used by a compiler that's different from the one that built glib itself.  This means that we can't use a configure check to detect it...

you can certainly add a #ifdef _GCC_ in that header, but whats the win for us ?

Probably we should check a specific GCC version...

The win is that the error message that we get is a lot more reasonable.

*** Bug 691107 has been marked as a duplicate of this bug. ***

(In reply to comment #1)
> This might be harder than it seems -- G_STATIC_ASSERT is in a public header
> which means it may end up being used by a compiler that's different from the
> one that built glib itself.  This means that we can't use a configure check to
> detect it...

That's no different than any of our other compile-tweaking macros. You just need to figure out what version of gcc it was introduced in, like:

#if    __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)
#define G_DEPRECATED_FOR(f) __attribute__((__deprecated__("Use '" #f "' instead")))
#else
#define G_DEPRECATED_FOR(f) G_DEPRECATED
#endif

A bit annoying to have it tied to GCC, but I guess it's better than the current situation (and I don't actually know that any non-GCC compiler would have _Static_assert except clang, which would claim to be GCC....).

In any case, looks like it came in 4.6.0.  I'll do a patch.

http://gcc.gnu.org/gcc-4.6/changes.html

Of course, it's more complicated...

There are a few potential issues.

As of 4.6.0, it seems that the feature can be optionally disabled -- so maybe a version check isn't enough, or maybe we should bump it up to a version where it's always on (which would take a bit more research).

Next, -pedantic complains about us using _Static_assert since it's not in ISO C.  We could possibly disable this by using expression-level pragmas to push error ignores as part of the macro, but that's sort of ugly.

Also, we introduce a small compatibility problem.

Since we used to use 'typedef' (a declaration) as the basis of G_STATIC_ASSERT, it was okay to mix it with other declarations.  _Static_assert() on the other hand is considered to be code, so (strictly speaking), we can't.  Again, this shows up with -pedantic.

I checked building glib with -pedantic.  There are a few warnings that are already there, but this would be the first warnings caused by using -pedantic and merely using glib's headers...  If our hope is to avoid causing trouble for people with weird compiler flags, we're probably going to have to try harder...

Created attachment 232740 [details] [review]
Use _Static_assert() from G_STATIC_ASSERT

GCC 4.6.0 introduced this, and without a better way to check for its
existence, we just check for this version of GCC.

(In reply to comment #7)
> Since we used to use 'typedef' (a declaration) as the basis of G_STATIC_ASSERT,
> it was okay to mix it with other declarations.  _Static_assert() on the other
> hand is considered to be code, so (strictly speaking), we can't.  Again, this
> shows up with -pedantic.

Also with various warning flags. That might be a showstopper.

So one way out of this may be to declare a new macro.  _Static_assert() takes a second parameter which is a string to describe the failed assertion -- our macro could have that.

Then we could port all of our own uses of it (in our public headers) to the new macro.

That would dodge the potential compatibility issues.

Sure, if somebody wants to write a patch that does that.

Created attachment 234231 [details] [review]
Use _Static_assert or static_assert and add G_STATIC_ASSERT2

gcc doesn't support _Static_assert for C++. If __cplusplus is >= 201103 it should use static_assert. Otherwise use the old implementation
I propose adding a macro G_STATIC_ASSERT2 which takes the message as a second argument. In case there is no way to show the message it should be ignored.

Created attachment 234475 [details] [review]
another patch

I hadn't seen your patch. Here is another one, based on gnulib's verify.h.
Not tested with gcc 4.8 or c++

Comment on attachment 234231 [details] [review]
Use _Static_assert or static_assert and add G_STATIC_ASSERT2

>+#    define G_STATIC_ASSERT(expr) _Static_assert(expr, "G_STATIC_ASSERT(" #expr ")")
>+#    define G_STATIC_ASSERT2(expr,message) _Static_assert(expr, message)

C11 allows _Static_assert() to be used inside a struct declaration, but the non-_Static_assert definition of G_STATIC_ASSERT() wouldn't work there. So if we define G_STATIC_ASSERT to just map directly to _Static_assert, then it would be possible to write code using it that compiled under gcc 4.6, but not under 4.5. The code that Matthias copied from gnulib avoids that trap, so let's go with his patch.

Comment on attachment 234475 [details] [review]
another patch

>This patch is inspired by gnulibs verify.h.

apostrophe

>At the same time, we introduce G_STATIC_ASSERT2 and
>G_STATIC_ASSERT_EXPR2 which take the diagnostic message
>as second argument.

There has to be a better name for those... G_STATIC_ASSERT_MESSAGE() ?

>+#ifdef _G_HAVE__STATIC_ASSERT
>+#define _G_VERIFY _Static_assert
...
>+#define G_STATIC_ASSERT(expr) _G_VERIFY (expr, "G_STATIC_ASSERT (" #expr ")")

Oops, I skimmed this patch too quickly before; it has the same issue with the macro being usable in different contexts depending on the gcc version. But I think you could just replace _G_VERIFY (in both the _G_HAVE__STATIC_ASSERT and not cases) with:

  #define _G_VERIFY(expr, msg) typedef _G_VERIFY_TYPE(expr, msg) _G_GENSYM

or something like that? I guess the gnulib version has the advantage of not triggering -Wunused-typedefs. Although it probaby triggers some other warning...

*** Bug 758844 has been marked as a duplicate of this bug. ***

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/620.