[lttng-dev] [PATCH 1/3] urcu: avoid "expression result unused" from clang
Mathieu Desnoyers
mathieu.desnoyers at efficios.com
Mon Dec 10 09:09:31 EST 2012
* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> On 12/08/2012 11:00 PM, Mathieu Desnoyers wrote:
> > * Lai Jiangshan (eag0628 at gmail.com) wrote:
> >> How about the one in yy.diff?
> >>
> >> xx.diff fixes the one what I had sent in 1/3 patch, but it may still
> >> cause warning in future if the complier become more strict.
> >
> > Hrm. In any cases, I think that even with the 2 patches you propose, the
> > warning "unused expression return" would still be valid (because we end
> > up not using the result of the outer statement-expression).
>
> The one in yy.diff:
>
> 1 ? _v : statement_which_has_side_affect;
>
> The compiler should not warn for it. "?:" can be used for control flow,
> the complier should only warn if both branches are warning.
In the case of yy.diff, whether the warning will appear or not would
depend on the phase at which the compiler generates the warning, no ?
For instance, it it chooses to check for unused return values after
eliminating dead code (considering branches that cannot be taken), then
we would be in the same situation as we are today, even if we have a
statement with side effect.
I am more and more enclined to try asking their opinion to the clang
community about this.
Thanks,
Mathieu
>
>
> We might
> > want to consider simpler approaches:
> >
> > 1 - Add the missing (void) casts before each CMM_STORE_SHARED() in the
> > code. I don't like this because it makes the CMM_STORE_SHARED common
> > case more cumbersome to use.
> > 2 - Change the CMM_STORE_SHARED() API so it does not return anything.
> > I think most use-cases don't involve using its return value anyway.
> >
> > Thoughts ?
> >
> > Thanks,
> >
> > Mathieu
> >
> >>
> >> On Sat, Dec 8, 2012 at 1:27 AM, Lai Jiangshan <eag0628 at gmail.com> wrote:
> >>>
> >>>
> >>> On Saturday, December 8, 2012, Mathieu Desnoyers wrote:
> >>>>
> >>>> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> >>>>> The last expression result is unused and clang will complain.
> >>>>> The trick in the patch supresses the complaint
> >>>>
> >>>> Hrm, but with this patch, gcc complains:
> >>>>
> >>>> rculfhash.c:1921:3: warning: variable '_w' set but not used
> >>>> [-Wunused-but-set-variable]
> >>>>
> >>>> using
> >>>>
> >>>> gcc version 4.7.2 (Debian 4.7.2-4)
> >>>
> >>>
> >>> Ouch, my gcc is too old
> >>>
> >>>
> >>>>
> >>>>
> >>>> Thoughts ?
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Mathieu
> >>>>
> >>>>>
> >>>>> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> >>>>> ---
> >>>>> urcu/system.h | 5 ++++-
> >>>>> 2 files changed, 4 insertions(+), 1 deletions(-)
> >>>>>
> >>>>> diff --git a/urcu/system.h b/urcu/system.h
> >>>>> index 2a45f22..6b7b0af 100644
> >>>>> --- a/urcu/system.h
> >>>>> +++ b/urcu/system.h
> >>>>> @@ -46,12 +46,15 @@
> >>>>> /*
> >>>>> * Store v into x, where x is located in shared memory. Performs the
> >>>>> * required cache flush after writing. Returns v.
> >>>>> + * "_w" here avoids the warning from clang:
> >>>>> + * warning: expression result unused [-Wunused-value]
> >>>>> */
> >>>>> #define CMM_STORE_SHARED(x, v) \
> >>>>> ({ \
> >>>>> __typeof__(x) _v = _CMM_STORE_SHARED(x, v); \
> >>>>> + __typeof__(x) _w; \
> >>>>> cmm_smp_wmc(); \
> >>>>> - _v; \
> >>>>> + _w = _v; \
> >>>>> })
> >>>>>
> >>>>> #endif /* _URCU_SYSTEM_H */
> >>>>> --
> >>>>> 1.7.4.4
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> lttng-dev mailing list
> >>>>> lttng-dev at lists.lttng.org
> >>>>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> >>>>
> >>>>
> >>>> --
> >>>> Mathieu Desnoyers
> >>>> Operating System Efficiency R&D Consultant
> >>>> EfficiOS Inc.
> >>>> http://www.efficios.com
> >>>>
> >>>> _______________________________________________
> >>>> lttng-dev mailing list
> >>>> lttng-dev at lists.lttng.org
> >>>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> >
> >
> >
> >
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list