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

Jeremy Manson jeremymanson at google.com
Mon Jun 3 15:57:45 PDT 2013


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>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 <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<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/6bfe70d8/attachment.html 


More information about the hotspot-runtime-dev mailing list