RFR 8014431 : cleanup warnings indicated by the -Wunused-value compiler option on linux

[Calvin Cheung]

The fix is pretty much ready and David H. has reviewed it.
I'll need one more review before the fix can be checked in.

Calvin
p.s. I was on vacation last week.

On 6/3/2013 3:57 PM, Jeremy Manson wrote:
> I can't really argue with you there!  What's the status of this 
> review, anyway?
>
>
> On Mon, Jun 3, 2013 at 12:04 AM, David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
>
>     On 30/05/2013 5:28 AM, Jeremy Manson wrote:
>
>         On Thu, May 23, 2013 at 5:56 PM, David Holmes
>         <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>         <mailto:david.holmes at oracle.com
>         <mailto:david.holmes at oracle.com>>> wrote:
>
>                     src/share/vm/utilities/__taskqueue.hpp
>
>
>                     I do not understand this code:
>
>                     ! // g++ complains if the volatile result of the
>         assignment
>                     is unused.
>                     ! const_cast<E&>(_elems[__localBot] = t);
>
>
>                     why do we even need the const cast? How is the
>         assignment
>                     not used ???
>
>                 I've tried without the const cast and got the
>         following error:
>                
>         /scratch/cccheung/hs25/src/__share/vm/utilities/taskqueue.__hpp:348:
>
>
>                 error:
>                 object of type ‘volatile StarTask&’ will not be
>         accessed in
>                 statement
>
>                 There was an original comment about the const cast as
>         follows:
>                 343 // g++ complains if the volatile result of the
>         assignment is
>                 unused.
>                 344 const_cast<E&>(_elems[__localBot] = t);
>
>
>
>             Yep I already quoted that comment above :) And I still don't
>             understand it. What variable is a "volatile E&"? _elems is a
>             "volatile E*"  so I guess _elems[i] is a "volatile E&" but
>         we are
>             assigning it, so how can it not be used ??? I wonder if
>         the problem
>             is that "t" is a plain E not a "volatile E&"? But then I
>         also wonder
>             whether _elems was meant to be declared as "E * volatile" ?
>
>             Anyway your current fix is fine.
>
>
>
>         I was away on vacation last week, so I didn't participate in this
>         thread.   Since most of the fixes here were mine, I should
>         probably
>         comment.  The "cast the volatile away" is because of the last
>         sentence
>         in this doc:
>
>         http://gcc.gnu.org/onlinedocs/gcc-4.0.4/gcc/Volatiles.html
>
>         The value returned by the assignment statement counts as a
>         read of a
>         const volatile variable.  Since the const volatile read is
>         unused, g++
>         complains.  The answer is to cast the value to an rvalue (in
>         this case,
>         the original programmer chose to cast away the const).
>
>
>     Thanks for the pointer. This seems like sheer silliness on the
>     part of gcc:
>
>     _elems[i];
>
>     should issue a warning
>
>     _elems[i] = t;
>
>     should not.
>
>     David
>
>
>
>         This is all well and good, but now we have a const_cast whose
>         value is
>         unused, so we have to cast it to void.
>
>         We can't cast directly to void because void isn't an rvalue.
>
>         I couldn't fit that entire explanation in the comment.  It
>         might be
>         worth someone else having a crack at it.
>
>         And it is very possible that the logic is wrong.  I didn't look
>         carefully at it, but relying on volatile in C++-not-11 to do
>         anything
>         other than ensure that writes to mmap regions all happen is
>         probably a
>         bad idea.
>
>         Jeremy
>
>         -
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130603/f24f9143/attachment.html