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

[Coleen Phillimore]

Calvin,
This looks good.   Thank you for sponsoring this change and to Jeremy 
for making it.

Thanks,
Coleen

On 06/03/2013 09:07 PM, Calvin Cheung wrote:
> 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/20130604/f5e751df/attachment-0001.html