Created attachment 371986 [details] [review]
patch

Thanks; alas I'm afraid we'll have to postpone it til next API breakage (soname bump).

This was done by design actually, so you can do

regex = vte_regex_unref(regex)

and thus NULL out the variable in one go.

(In reply to Igor from comment #0)
> vte_regex_unref() is declared to return VteRegex* which is unnecessary and
> produces function type cast warnings.
> The proposed patch makes it return void which eliminates the warnings.

What's the warning exactly?

I'd say ideally should follow existing practice, e.g. void g_object_unref; but once done differently, I'm not sure if it's worth the hassle to change it.

(In reply to Christian Persch from comment #4)
> What's the warning exactly?

There are 2 of them that are repeated like 20 times when building vte on an up-to-date Arch system with gcc 8.1.0:

/usr/include/vte-2.91/vte/vteregex.h: In function ‘glib_listautoptr_cleanup_VteRegex’:

/usr/include/glib-2.0/glib/gmacros.h:462:99: warning: cast between incompatible function types from ‘VteRegex * (*)(VteRegex *)’ {aka ‘struct _VteRegex * (*)(struct _VteRegex *)’} to ‘void (*)(void *)’ [-Wcast-function-type]

static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \

^

/usr/include/vte-2.91/vte/vteregex.h:71:1: note: in expansion of macro ‘G_DEFINE_AUTOPTR_CLEANUP_FUNC’

G_DEFINE_AUTOPTR_CLEANUP_FUNC(VteRegex, vte_regex_unref)

^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/usr/include/vte-2.91/vte/vteregex.h: In function ‘glib_slistautoptr_cleanup_VteRegex’:

/usr/include/glib-2.0/glib/gmacros.h:463:102: warning: cast between incompatible function types from ‘VteRegex * (*)(VteRegex *)’ {aka ‘struct _VteRegex * (*)(struct _VteRegex *)’} to ‘void (*)(void *)’ [-Wcast-function-type]

static inline void _GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName) (GSList **_l) { g_slist_free_full (*_l, (GDestroyNotify) func); } \

^

/usr/include/vte-2.91/vte/vteregex.h:71:1: note: in expansion of macro ‘G_DEFINE_AUTOPTR_CLEANUP_FUNC’

G_DEFINE_AUTOPTR_CLEANUP_FUNC(VteRegex, vte_regex_unref)

^~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Moreover, when building an application depending on vte, like xfce4-terminal, the compiler is producing a bunch of similar warnings.

(In reply to Christian Persch from comment #3)
> This was done by design actually, so you can do
> regex = vte_regex_unref(regex)
> and thus NULL out the variable in one go.

Well, as far as I can tell all other unref functions return void...

Does changing 

G_DEFINE_AUTOPTR_CLEANUP_FUNC(VteRegex, vte_regex_unref)

to add a cast before the vte_regex_unref() work?

In fact, changing it as
G_DEFINE_AUTOPTR_CLEANUP_FUNC(VteRegex, (GDestroyNotify) vte_regex_unref)
adds even more warnings.

Also, it seems type casting function pointers is undefined behavior in C:
https://stackoverflow.com/questions/559581/casting-a-function-pointer-to-another-type
https://stackoverflow.com/questions/188839/function-pointer-cast-to-different-signature

That may be UB, but glib relies on this anyway, so that's not a problem.

Would the

#pragma GCC diagnostic push
#if G_GNUC_CHECK_VERSION(major, minor)
#pragma GCC diagnostic ignored "-Wcast-function-type"
#endif
G_DEFINE_AUTOPTR_CLEANUP_FUNC(...)
#pragma GCC diagnostic pop

dance fix this? (need to find out which gcc version added this -W flag).

Created attachment 372034 [details] [review]
patch v2

gcc 8 add this flag to -Wextra: https://gcc.gnu.org/gcc-8/changes.html

#if GLIB_CHECK_VERSION(2, 44, 0)
#pragma GCC diagnostic push
#if __GNUC__ >= 8
#pragma GCC diagnostic ignored "-Wcast-function-type"
#endif
G_DEFINE_AUTOPTR_CLEANUP_FUNC(VteRegex, vte_regex_unref)
#pragma GCC diagnostic pop
#endif

does the job of suppressing the warnings.
I've uploaded a new patch for the code above. I've also suppressed the warning in another place where vte_regex_unref() gets cast to GBoxedFreeFunc.

It seems that some other Gnome projects have also done similar suppressions, see e.g. https://gitlab.gnome.org/GNOME/gjs/commit/5aba4c855ae7a72567ac83304e407b729f8018ca

This will be fixed in glib, bug 796237  -> closing.

*** This bug has been marked as a duplicate of bug 796237 ***