* Lai Jiangshan ([email protected]) wrote: > On 12/08/2012 11:00 PM, Mathieu Desnoyers wrote: > > * Lai Jiangshan ([email protected]) 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 <[email protected]> wrote: > >>> > >>> > >>> On Saturday, December 8, 2012, Mathieu Desnoyers wrote: > >>>> > >>>> * Lai Jiangshan ([email protected]) 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 <[email protected]> > >>>>> --- > >>>>> 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 > >>>>> [email protected] > >>>>> 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 > >>>> [email protected] > >>>> 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 [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
