On Fri, 2021-09-10 at 13:33 +0100, Daniel P. Berrangé wrote: > On Fri, Sep 10, 2021 at 02:26:32PM +0200, Tim Wiederhake wrote: > > On Fri, 2021-09-10 at 11:53 +0100, Daniel P. Berrangé wrote: > > > On Fri, Sep 10, 2021 at 12:45:43PM +0200, Tim Wiederhake wrote: > > > > Workaround for a bug in clang. Clang emits an unused-variable > > > > warning > > > > if the variable is only accessed on scope exit by a destructor > > > > function. > > > > Note that gcc does not exhibit this behavior. > > > > > > > > See https://bugs.llvm.org/show_bug.cgi?id=3888 and > > > > https://bugs.llvm.org/show_bug.cgi?id=43482. > > > > > > > > Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx> > > > > --- > > > > src/util/glibcompat.h | 19 +++++++++++++++++++ > > > > 1 file changed, 19 insertions(+) > > > > > > > > diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h > > > > index 697687b967..ea486e32d5 100644 > > > > --- a/src/util/glibcompat.h > > > > +++ b/src/util/glibcompat.h > > > > @@ -86,3 +86,22 @@ char *vir_g_strdup_vprintf(const char *msg, > > > > va_list args) > > > > #define g_fsync vir_g_fsync > > > > > > > > void vir_g_source_unref(GSource *src, GMainContext *ctx); > > > > + > > > > +/* > > > > + * Workaround for a bug in clang. Clang emits an unused-variable > > > > warning if the > > > > + * variable is only accessed on scope exit by a destructor > > > > function. See > > > > + * https://bugs.llvm.org/show_bug.cgi?id=3888 and > > > > + * https://bugs.llvm.org/show_bug.cgi?id=43482. > > > > + */ > > > > +#if defined(__clang__) > > > > +# undef g_autoptr > > > > +# define g_autoptr(TypeName) > > > > _GLIB_CLEANUP(_GLIB_AUTOPTR_FUNC_NAME(TypeName)) > > > > _GLIB_AUTOPTR_TYPENAME(TypeName) G_GNUC_UNUSED > > > > +# undef g_autolist > > > > +# define g_autolist(TypeName) > > > > _GLIB_CLEANUP(_GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName)) > > > > _GLIB_AUTOPTR_LIST_TYPENAME(TypeName) G_GNUC_UNUSED > > > > +# undef g_autoslist > > > > +# define g_autoslist(TypeName) > > > > _GLIB_CLEANUP(_GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName)) > > > > _GLIB_AUTOPTR_SLIST_TYPENAME(TypeName) G_GNUC_UNUSED > > > > +# undef g_autoqueue > > > > +# define g_autoqueue(TypeName) > > > > _GLIB_CLEANUP(_GLIB_AUTOPTR_QUEUE_FUNC_NAME(TypeName)) > > > > _GLIB_AUTOPTR_QUEUE_TYPENAME(TypeName) G_GNUC_UNUSED > > > > +# undef g_auto > > > > +# define g_auto(TypeName) > > > > _GLIB_CLEANUP(_GLIB_AUTO_FUNC_NAME(TypeName)) TypeName > > > > G_GNUC_UNUSED > > > > +#endif /* __clang__ */ > > > > > > This patch should not be needed anymore afaik since we addressed > > > the > > > problems that were triggering clang. > > > > > > > Did you see patches 9 to 12? They introduce more "only used on scope > > exit" variables that trigger clangs warning otherwise. > > Why aren't they following the approach that Peter used to address this > though, by adding the G_GNUC_UNUSED annotations to the usage sites ? > Adding G_GNUC_UNUSED at the usage sites (of which there will be a couple hundred), will disable unused variable detection irregardless of used compiler, just to work around a bug in one single compiler. I would rather use "G_GNUC_UNUSED" only for variables / function arguments that actually are unused. The lock guard variables are used, but at the end of the scope rather than during. > We should only re-define the g_auto* macros if we have first submitted > this change to GLib upstream and they merged it. > What are your thoughts on macros (#if/#else'd over __clang__ to include G_GNUC_UNUSED as required) like this? #define VIR_LOCK_GUARD_MUTEX(mutex) \ g_autoptr(virLockGuard) G_GNUC_UNUSED lockguard ## __COUNTER__ = virLockGuardNew(mutex); #define VIR_LOCK_GUARD_OBJECT(object) \ g_autoptr(virLockGuard) G_GNUC_UNUSED lockguard ## __COUNTER__ = virObjectLockGuard(object); or: #define VIR_LOCK_GUARD_MUTEX g_autoptr(virLockGuard) G_GNUC_UNUSED VIR_LOCK_GUARD_MUTEX lock1 = virLockGuardNew(mutex); VIR_LOCK_GUARD_MUTEX lock2 = virObjectLockGuard(object); Regards, Tim > Regards, > Daniel
