[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: RFR 8014431 : cleanup warnings indicated by the -Wunused-value compiler option on linux
From: Jeremy Manson <jeremymanson () google ! com>
Date: 2013-05-29 19:28:16
Message-ID: CAPYFHW0tecQo0zoFSXMx69SJ=jBdVEzXj9=2DOLf4FSTHOxNNw () mail ! gmail ! com
[Download RAW message or body]
On Thu, May 23, 2013 at 5:56 PM, David Holmes <david.holmes@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).
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
-
[Attachment #3 (text/html)]
<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, \
May 23, 2013 at 5:56 PM, David Holmes <span dir="ltr"><<a \
href="mailto:david.holmes@oracle.com" \
target="_blank">david.holmes@oracle.com</a>></span> wrote:<blockquote \
class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">
<blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><blockquote \
class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
src/share/vm/utilities/<u></u>taskqueue.hpp<br>
<br>
I do not understand this code:<br>
<br>
! // g++ complains if the volatile result of the assignment is unused.<br>
! const_cast<E&>(_elems[<u></u>localBot] = t);<br>
<br>
why do we even need the const cast? How is the assignment not used ???<br>
</blockquote>
I've tried without the const cast and got the following error:<br>
/scratch/cccheung/hs25/src/<u></u>share/vm/utilities/taskqueue.<u></u>hpp:348: \
error:<br> object of type ‘volatile StarTask&’ will not be accessed in \
statement<br> <br>
There was an original comment about the const cast as follows:<br>
343 // g++ complains if the volatile result of the assignment is unused.<br>
344 const_cast<E&>(_elems[<u></u>localBot] = t);<br>
</blockquote>
<br></div>
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" ?<br>
<br>
Anyway your current fix is fine.</blockquote><div><br></div><div><br></div><div \
style>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:</div>
<div style><br></div><div style><a \
href="http://gcc.gnu.org/onlinedocs/gcc-4.0.4/gcc/Volatiles.html">http://gcc.gnu.org/onlinedocs/gcc-4.0.4/gcc/Volatiles.html</a><br></div><div \
style><br></div><div style>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).</div>
<div style><br></div><div style>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.</div><div \
style><br></div><div style>We can't cast directly to void because void isn't \
an rvalue.</div>
<div style><br></div><div style>I couldn't fit that entire explanation in the \
comment. It might be worth someone else having a crack at it.</div><div \
style><br></div><div style>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.</div>
<div style><br></div><div style>Jeremy</div><div style><br></div><div style>- \
</div></div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic