Incorrect syntax/warning highlight, EAP 992

EAP 992
Solaris 8
J2SDK 1.4.2_03

The IDE is highlighting the last line of the following fragment as a warning:

int index = 0;
foo( ++index );
foo( ++index );

The reported warning is: "The value changed at ++index is never used"

Nick

17 comments

Similar issues have been discussed before. Idea is right, however:
If you don't use index anywhere else after the last call to foo, then that call can be replaced with

0

There is a bug report for this (http://www.intellij.net/tracker/idea/viewSCR?publicId=11942), but unfortunately it's state is on "do not fix". Stephen is right, precisely speaking it is in fact an unused assignment. But from my point of view that's too nitpicking - and it's particularly annoying. If you have to add another similar line in your example (or remove the last line) you can't simply duplicate the last line but you have to change the last two lines every time.

0

On Thu, 11 Dec 2003 08:48:00 +0000 (UTC), Stephen Kelvin
<mail@gremlin.info> wrote:

Similar issues have been discussed before. Idea is right, however:
If you don't use index anywhere else after the last call to foo, then
that call can be replaced with



I would agree if it said foo(index+), but in the case of foo(+index) the
changed value is used.

--

- Rene Smit

0

This looks like a bug to me - it's the difference between prefix and
postfix, right? I mean, I would expect to see the warning if I
foo( index++ );
foo( index++ );
because the last value is indeed not used. But when I do
foo( ++index );
then the statement is equivalent to
index = index+1;
foo( index );
and here obviously the value is used.

Robbie

Nick Pratt wrote:

EAP 992
Solaris 8
J2SDK 1.4.2_03

The IDE is highlighting the last line of the following fragment as a warning:

int index = 0;
foo( ++index );
foo( ++index );

The reported warning is: "The value changed at ++index is never used"

Nick


0

I would agree if it said foo(index++), but in the
case of foo(++index) the
changed value is used.


Unfortunately you're wrong. The code

is equivalent to

It is pretty obvious that the second statement is superfluous.

But I agree that flagging this case as a warning is very nitpicking and annoys more than it helps finding errors.

0

...because the last value is indeed not used. But when I
do
foo( ++index );
then the statement is equivalent to
index = index+1;
foo( index );
and here obviously the value is used.


You too are wrong: As stated in my other post the code

is equivalent to

where the second statement is useless.

The difference is that by writing foo(++index) the result of the addition is stored in the local variable array, but by writing foo(index + 1) the addition is done solely on the operand stack of the VM.

Yes, you're right, this is an abosutely minor difference, and therefore I would like IDEA not to show it as a warning. But nevertheless it's correct and someone very nitpicking could treat it as a flaw.

0


Unfortunately you're wrong. The code


is equivalent to

 index = index + 1;]]>


Unfortunately you are wrong yourself. Consider the following code and you will see the difference.


Regards,
Jens

0

Unfortunately you're wrong yourself.


Hi Martin,

I take this back; there appears to be a difference between member fields and local variables, so you are probably right.

Regards,
Jens

0

Sorry that I have to correct you. You're example is in no way descriptive since you pass a variable to method foo which is never used there. And in addition the line foo(index) is not marked as warning in your example, even if it's the last line. Instead the line static void foo(int a) is correctly marked as warning (Parameter a is never used).

We are talking here about pre-incrementing a local variable, not a member field.

0

Martin Fuhrer wrote:
>>...because the last value is indeed not used. But when I
>>do
>>foo( ++index );
>>then the statement is equivalent to
>>index = index+1;
>>foo( index );
>>and here obviously the value is used.


You too are wrong: As stated in my other post the code


is equivalent to

 index = index + 1;]]>

where the second statement is useless.

The difference is that by writing foo(++index) the result of the addition is stored in the local variable array, but by writing foo(index + 1) the addition is done solely on the operand stack of the VM.

Yes, you're right, this is an abosutely minor difference, and therefore I would like IDEA not to show it as a warning. But nevertheless it's correct and someone very nitpicking could treat it as a flaw.


Are you sure about this? I checked the JLS, section 15.15.1:
"The value of the prefix increment expression is the value of the
variable after the new value is stored."
I read that to mean that the assignment is done before the expression is
evaluated. It doesn't say "The value of the prefix increment expression
is the value that the variable will have after the new value is stored."

What about this:
foo(++index, index);
Are you saying that it is actually
foo(index1, index1);
index = index+1;
???
This may well be how it works out at bytecode level due to register
optimisations etc., but it makes no sense to me to have Idea telling me
that the new value of index is never used in the expression
foo(++index);
because the JLS tells me that it is used and I don't work at a bytecode
level, me!

Robbie

0

Robert,
Martin is absolutely right but maybe not exactly precise
call foo(++index);

can be replaced exactly by
index += 1;
foo (index);

because call to the foo is absolutely last operator in the function body new
value of index is never used in the function itself only in the stack of
method foo so more correct will be simply to replace previous two lines with
one:
foo (index + 1);
But be careful because everything above mentioned is true only for index
being a local variable. How important or unimportant this warning is
actually debatable but I would note that JVM generates different byte code
in those two cases.

Thank you,
Alex Oscherov

"Robert Gibson" <robbie_usenet@yahoo.co.uk> wrote in message
news:bra7iv$j8r$1@is.intellij.net...

Martin Fuhrer wrote:
>>...because the last value is indeed not used. But when I
>>do
>>foo( ++index );
>>then the statement is equivalent to
>>index = index+1;
>>foo( index );
>>and here obviously the value is used.
>
>

You too are wrong: As stated in my other post the code


is equivalent to

 > index = index + 1;]]>

where the second statement is useless.

>

The difference is that by writing foo(++index) the result of the

addition is stored in the local variable array, but by writing foo(index +
1) the addition is done solely on the operand stack of the VM.

>

Yes, you're right, this is an abosutely minor difference, and therefore

I would like IDEA not to show it as a warning. But nevertheless it's correct
and someone very nitpicking could treat it as a flaw.
>

Are you sure about this? I checked the JLS, section 15.15.1:
"The value of the prefix increment expression is the value of the
variable after the new value is stored."
I read that to mean that the assignment is done before the expression is
evaluated. It doesn't say "The value of the prefix increment expression
is the value that the variable will have after the new value is stored."

>

What about this:
foo(++index, index);
Are you saying that it is actually
foo(index1, index1);
index = index+1;
???
This may well be how it works out at bytecode level due to register
optimisations etc., but it makes no sense to me to have Idea telling me
that the new value of index is never used in the expression
foo(++index);
because the JLS tells me that it is used and I don't work at a bytecode
level, me!

>

Robbie

>


0

Its more a hassle of seeing a warning in the editor, than the syntax.

We use the ++index construct all over the place for large ( and changing ) prepared SQL statements and stored procedures.

As has been mentioned, when I see foo(++index); I consider that "index" has been used.

Nick

0

Ok, let's try to make things clear:

1.) You're absolutely right. The warning is useless, senseless, and doesn't improve the code in any way.
-> 1 point against IDEA

2.) Me personally I hate this warning. It warns me about something which I consider being absolutely correct and which doesn't do any harm. If I modify my code to avoid the warning the code doesn't get better but less maintainable.
-> (personal opinion, no points here)

3.) From a somewhat philosophical point of view IDEA is right: there's an expression modifying a local variable which is never used after this same expression. So therefore it's an unused assignment.
-> 1 point for IDEA

4.) An expression like foo(idx) is compiled into these bytecode instructions:
- increase value of local variable by one
- load local variable onto operand stack
- call the method
So exactly spoken the variable is in fact used after assigning it a new value.
-> 1 point against IDEA

5.) Several people (including me) have reported this warning as a bug. So it is pretty obvious that it
doesn't match most peoples expectations.
-> 1 point against IDEA

So in the end its 3:1 against IDEA, i.e. in my opinion it should still be treated as a bug. Unfortunately JetBrains have decided point 3 being the most important and have closed the bug as "do not fix".

0


Your point 4 is technically incorrect. According to a close reading of the JLS, a Java compiler would be totally within it's rights to produce bytecode which didn't increment the local variable, if it could prove that that local variable was never accessed. None of the current java compilers perform such optimization, leaving that to the JIT, but they would be correct to do so if they wished. (I've seen benchmarks indicating that all of the modern JITs include optimizations which do remove the write-back to the local variable). I'd call the score 2-to-2, rather than 3-to-1. Then again, I personally never use the increment or decrement operators as anything other than standalone statements, so it's not like I've got a stake in this game.

--Dave

0

(I've seen benchmarks indicating that all of the
modern JITs include optimizations which do remove the
write-back to the local variable).


I examined the bytecode produced by the 1.4.1_02 compiler. There the local variable isn't written back but is incremented in place (using the iinc instruction).

Instead the call +foo(idx + 1)+ is translated into
- load variable onto operand stack
- place constant 1 onto operand stack
- add the two operands
- call the method

I doubt that the second version is measurable faster than the first, even if the first has to modify a local variable (remember: we're not talking about a member field).

Of course compilers are free to produce different bytecode, but using the iinc instruction for an prefix or postfix increment expression seems not too silly to me.

And if some compiler or JIT optimizes the code by don't actually storing the result - why should IDEA then warn about an usused assigment if there's no assignment at all?

So I'm still on my 3-to-1 score.

...so it's not like I've got a stake in this game.


Thanks anyway for giving some interesting hints.

0

Which bit of the JLS are you reading closely? I'm looking at section
15.15.1 of the JLS, second edition
http://java.sun.com/docs/books/jls/second_edition/html/expressions.doc.html#39547
and it says
"... the value 1 is added to the value of the variable and the sum is
stored back into the variable ... The value of the prefix increment
expression is the value of the variable after the new value is stored."

I read that to mean that the variable is modified and then that value is
used. Of course a JVM is free to optimise it however it wants, but we
don't program in bytecode, we program in Java - the two are different
languages, albeit strongly related :)

So I have the score at 3-0 against Idea (since I don't count bytecode
analysis either way).

Robbie

0

Please sign in to leave a comment.