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

David Holmes david.holmes at oracle.com
Mon Jun 3 00:04:48 PDT 2013


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>> 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
>
> -


More information about the hotspot-runtime-dev mailing list