www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Warn about do nothing expressions?

reply Benjamin Thaut <code benjamin-thaut.de> writes:
I had a bug which came down to the following line today:

m_takeIndex = m_takeIndex++;

Actually this line does nothing. m_takeIndex remains at exactly the same 
value as before (tested with dmd 2.065).

Can someone please explain why? And would be suitable to warn about 
this, if it is correct behaviour?

Kind Regards
Benjamin Thaut
Mar 28 2014
next sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Friday, 28 March 2014 at 16:54:49 UTC, Benjamin Thaut wrote:

 I had a bug which came down to the following line today:

 m_takeIndex = m_takeIndex++;

 Actually this line does nothing. m_takeIndex remains at exactly 
 the same value as before (tested with dmd 2.065).

 Can someone please explain why?


IT does nothing because that's post-increment: EG "save the 
current value, increment, return the old value".

Then you assign back the old value, effectively putting you back 
where you started:

int i = 5;

i = i++; //i is 5
then
i = 5; //i was incremented to 6
then
i is 5 again;


 And would be suitable to warn about this, if it is correct 
 behaviour?


The problem here though is that we are talking about a "logical" 
no side effect: It *does* something, you just can't observe it.

Imagine this:

//----
void fun(ref i, ref j)
{
     i = j++;
}
void main()
{
     int a = 5;
     fun(a, a); //do nothing?
}
//----

This is to contrast with "i + j;" which is an *actual* no op.
Mar 28 2014
parent Benjamin Thaut <code benjamin-thaut.de> writes:
Am 28.03.2014 18:03, schrieb monarch_dodra:

 On Friday, 28 March 2014 at 16:54:49 UTC, Benjamin Thaut wrote:

 I had a bug which came down to the following line today:

 m_takeIndex = m_takeIndex++;

 Actually this line does nothing. m_takeIndex remains at exactly the
 same value as before (tested with dmd 2.065).

 Can someone please explain why?


 IT does nothing because that's post-increment: EG "save the current
 value, increment, return the old value".

 Then you assign back the old value, effectively putting you back where
 you started:

 int i = 5;

 i = i++; //i is 5
 then
 i = 5; //i was incremented to 6
 then
 i is 5 again;


 And would be suitable to warn about this, if it is correct behaviour?


 The problem here though is that we are talking about a "logical" no side
 effect: It *does* something, you just can't observe it.

 Imagine this:

 //----
 void fun(ref i, ref j)
 {
      i = j++;
 }
 void main()
 {
      int a = 5;
      fun(a, a); //do nothing?
 }
 //----

 This is to contrast with "i + j;" which is an *actual* no op.


Well when I mean warning, I really only mean the

var = var++

case without any indirections. Because I can't think of a case where you 
actually want this.

Kind Regards
Benjamin Thaut
Mar 28 2014
prev sibling parent reply "Frustrated" <Who where.com> writes:
On Friday, 28 March 2014 at 16:54:49 UTC, Benjamin Thaut wrote:

 I had a bug which came down to the following line today:

 m_takeIndex = m_takeIndex++;

 Actually this line does nothing. m_takeIndex remains at exactly 
 the same value as before (tested with dmd 2.065).

 Can someone please explain why? And would be suitable to warn 
 about this, if it is correct behaviour?

 Kind Regards
 Benjamin Thaut


This should be invalid.

m_takeIndex++;

actually increments the variable.

m_takeIndex = m_takeIndex++; should do exactly the same.

It is a bug. It might be correct in monarch's world but it is not 
logical. I think monarch is just trying to justify the way D does 
it, regardless if D is D is wrong or not.

I imagine what is going on is that D is creating a temp variable, 
assigning it to the local variable, then incrementing the temp 
variable(since++ increments "after" the assignment).

That, or D really is treating that as a nop, which is wrong too.

Test case shows it is probably the first:

	int j = 0, i = 0;
	i = j++;
	// i = 0, j = 1

hence D is wrong, Monarch is wrong, etc...

m_takeIndex = m_takeIndex++;

should, given the example above, evaluate to

m_takeIndex = m_takeIndex; (a nop)
m_takeIndex++; (variable incremented)


hence, D is not consistent with itself, which at the very least 
is a bug.
Mar 28 2014
next sibling parent "Barry L." <barry.lapthorn gmail.com> writes:
I ran this:

import std.stdio;

void main() {
     int i = 10;
     i = i++;
     writeln(i);
}

with DMD32 D Compiler v2.063.2 and got '10'.

And for nothing other than curiousity, this in both VS2012, and 
VS2013:

#include <iostream>

int main(int argc, char const *argv[])
{
     int i = 10;
     i = i++;
     std::cout << i << std::endl;
     return 0;
}

and got '11', both times.





On Friday, 28 March 2014 at 18:04:41 UTC, Frustrated wrote:

 On Friday, 28 March 2014 at 16:54:49 UTC, Benjamin Thaut wrote:

 I had a bug which came down to the following line today:

 m_takeIndex = m_takeIndex++;

 Actually this line does nothing. m_takeIndex remains at 
 exactly the same value as before (tested with dmd 2.065).

 Can someone please explain why? And would be suitable to warn 
 about this, if it is correct behaviour?

 Kind Regards
 Benjamin Thaut


 This should be invalid.

 m_takeIndex++;

 actually increments the variable.

 m_takeIndex = m_takeIndex++; should do exactly the same.

 It is a bug. It might be correct in monarch's world but it is 
 not logical. I think monarch is just trying to justify the way 
 D does it, regardless if D is D is wrong or not.

 I imagine what is going on is that D is creating a temp 
 variable, assigning it to the local variable, then incrementing 
 the temp variable(since++ increments "after" the assignment).

 That, or D really is treating that as a nop, which is wrong too.

 Test case shows it is probably the first:

 	int j = 0, i = 0;
 	i = j++;
 	// i = 0, j = 1

 hence D is wrong, Monarch is wrong, etc...

 m_takeIndex = m_takeIndex++;

 should, given the example above, evaluate to

 m_takeIndex = m_takeIndex; (a nop)
 m_takeIndex++; (variable incremented)


 hence, D is not consistent with itself, which at the very least 
 is a bug.
Mar 28 2014
prev sibling next sibling parent reply "QAston" <qaston gmail.com> writes:
On Friday, 28 March 2014 at 18:04:41 UTC, Frustrated wrote:

 On Friday, 28 March 2014 at 16:54:49 UTC, Benjamin Thaut wrote:

 I had a bug which came down to the following line today:

 m_takeIndex = m_takeIndex++;

 Actually this line does nothing. m_takeIndex remains at 
 exactly the same value as before (tested with dmd 2.065).

 Can someone please explain why? And would be suitable to warn 
 about this, if it is correct behaviour?

 Kind Regards
 Benjamin Thaut


 This should be invalid.

 m_takeIndex++;

 actually increments the variable.

 m_takeIndex = m_takeIndex++; should do exactly the same.

 It is a bug. It might be correct in monarch's world but it is 
 not logical. I think monarch is just trying to justify the way 
 D does it, regardless if D is D is wrong or not.

 I imagine what is going on is that D is creating a temp 
 variable, assigning it to the local variable, then incrementing 
 the temp variable(since++ increments "after" the assignment).

 That, or D really is treating that as a nop, which is wrong too.

 Test case shows it is probably the first:

 	int j = 0, i = 0;
 	i = j++;
 	// i = 0, j = 1

 hence D is wrong, Monarch is wrong, etc...

 m_takeIndex = m_takeIndex++;

 should, given the example above, evaluate to

 m_takeIndex = m_takeIndex; (a nop)
 m_takeIndex++; (variable incremented)


 hence, D is not consistent with itself, which at the very least 
 is a bug.


Monarch is right, expression value of i++ is the old i value. 
Right side of = is evaluated first, so i is incremented and then 
= is evaluated so it gets it's old value back. This is consistent 
with the rest of the language, your proposal is a special case.
Mar 28 2014
parent reply "Frustrated" <Who where.com> writes:
On Friday, 28 March 2014 at 18:29:27 UTC, QAston wrote:

 On Friday, 28 March 2014 at 18:04:41 UTC, Frustrated wrote:

 On Friday, 28 March 2014 at 16:54:49 UTC, Benjamin Thaut wrote:

 I had a bug which came down to the following line today:

 m_takeIndex = m_takeIndex++;

 Actually this line does nothing. m_takeIndex remains at 
 exactly the same value as before (tested with dmd 2.065).

 Can someone please explain why? And would be suitable to warn 
 about this, if it is correct behaviour?

 Kind Regards
 Benjamin Thaut


 This should be invalid.

 m_takeIndex++;

 actually increments the variable.

 m_takeIndex = m_takeIndex++; should do exactly the same.

 It is a bug. It might be correct in monarch's world but it is 
 not logical. I think monarch is just trying to justify the way 
 D does it, regardless if D is D is wrong or not.

 I imagine what is going on is that D is creating a temp 
 variable, assigning it to the local variable, then 
 incrementing the temp variable(since++ increments "after" the 
 assignment).

 That, or D really is treating that as a nop, which is wrong 
 too.

 Test case shows it is probably the first:

 	int j = 0, i = 0;
 	i = j++;
 	// i = 0, j = 1

 hence D is wrong, Monarch is wrong, etc...

 m_takeIndex = m_takeIndex++;

 should, given the example above, evaluate to

 m_takeIndex = m_takeIndex; (a nop)
 m_takeIndex++; (variable incremented)


 hence, D is not consistent with itself, which at the very 
 least is a bug.


 Monarch is right, expression value of i++ is the old i value. 
 Right side of = is evaluated first, so i is incremented and 
 then = is evaluated so it gets it's old value back. This is 
 consistent with the rest of the language, your proposal is a 
 special case.


Nope. Simple wrong. You even state what is happening
contradictory to what actually happens.

i = i++;
vs
i++;

different results. They shouldn't be.

i = i++; could be

i = i;
i++;

or

i++;
i = i;

or

i++;

either way, all increment i, which actually never happens in D.
As was pointed out, VS does it properly... D does it wrong.
Accept it and stop trying to validate how D does it so you can
say D is correct.

Not only is it logically wrong, it is not consistent with
previous interpretations of other compilers/languages nor with
itself. It is wrong on all levels. Just because you believe in
unicorns doesn't prove that they exist. All the evidence says you
are wrong.


What you guys are failing to realize is that

j = i;
i = j++;

is not the same as

i = i;
i = i++;

D also does not make the distinction, hence the error.

i = j++; is logically equivalent to

i = j;
j++;

which means, i = is the previous value of j(before increment).
This should hold true when j is an alias for i.

i = i;
i++;

which means i should be incremented. It is not. Why? Did monarch
code the increment routines in D?

I just wish we could agree that unicorns don't exist.
Mar 28 2014
next sibling parent reply "MattCoder" <somekindofmonster email.com.br> writes:
On Friday, 28 March 2014 at 19:35:22 UTC, Frustrated wrote:

 which means, i = is the previous value of j(before increment).
 This should hold true when j is an alias for i.


In this case: i = i++;  Means:

"i" is incremented by one, but the value assigned in "i" is the
previous one. That's it.

For me there's nothing wrong here.

Matheus.
Mar 28 2014
parent reply "Frustrated" <Who where.com> writes:
On Friday, 28 March 2014 at 19:57:13 UTC, MattCoder wrote:

 On Friday, 28 March 2014 at 19:35:22 UTC, Frustrated wrote:

 which means, i = is the previous value of j(before increment).
 This should hold true when j is an alias for i.


 In this case: i = i++;  Means:

 "i" is incremented by one, but the value assigned in "i" is the
 previous one. That's it.

 For me there's nothing wrong here.

 Matheus.



One thing you are getting wrong/leaving out is that i++, even in 
an assignment, increments i.

i = j++;

even if i is assigned the previous value of j, j STILL gets 
incremented by one.

Change i to j and it will work out.

Only if a temp value can the D behavior work as it does. This is 
wrong though because not all compilers have to use temp values.

Again, it is also inconsistent with the fact that something 
should get incremented.

"If ++ follows the variable, e.g. counter++, the value returned 
is the value in counter before it has been incremented. "

i = i++;
=> i = i; // value before i is incremented
    i++;   // now it is incremented.

which increments i due to last statement.

You can't increment i until you return the value.


mov ax, [i]  // put i in a register
mov ax, ax   // i = i // easily optimized out
inc ax       // i++
mov [i], ax

vs

mov ax, [i]
mov bx, ax
mov ax, bx
inc bx
mov [i], ax

I think most people would agree that the 2nd case is worse(one 
extra register is used for no real reason except to avoid the 
increment).
Mar 29 2014
next sibling parent Timon Gehr <timon.gehr gmx.ch> writes:
On 03/29/2014 05:31 PM, Frustrated wrote:

 .. i++, even in an assignment, increments i.
 ...


TDPL specifies the behaviour of i++ as follows:

{auto tmp=i; ++i; return tmp;}();


 i = j++;
 ...


i = {auto tmp=i; ++j; return tmp;}();


 even if i is assigned the previous value of j, j STILL gets incremented
 by one.

 Change i to j and it will work out.
 ...


i = {auto tmp=i; ++i; return tmp;}();
Mar 29 2014
prev sibling parent reply "MattCoder" <somekindofmonster email.com.br> writes:
On Saturday, 29 March 2014 at 16:31:48 UTC, Frustrated wrote:

 On Friday, 28 March 2014 at 19:57:13 UTC, MattCoder wrote:

 On Friday, 28 March 2014 at 19:35:22 UTC, Frustrated wrote:

 which means, i = is the previous value of j(before increment).
 This should hold true when j is an alias for i.


 In this case: i = i++;  Means:

 "i" is incremented by one, but the value assigned in "i" is the
 previous one. That's it.

 For me there's nothing wrong here.

 Matheus.



 One thing you are getting wrong/leaving out is that i++, even 
 in an assignment, increments i.


I see your pain pal... so I took a look around:
http://dlang.org/expression.html

And according the page above:

"Expressions

C and C++ programmers will find the D expressions very familiar,
with a few interesting additions.

Expressions are used to compute values with a resulting type.
These values can then be assigned, tested, or ignored.
Expressions can also have side effects.

Order Of Evaluation

The following binary expressions are evaluated in strictly
left-to-right order:
[i]
OrExpression, XorExpression, AndExpression, CmpExpression,
ShiftExpression, AddExpression, CatExpression, MulExpression,
PowExpression, CommaExpression, OrOrExpression, AndAndExpression
[/i]

The following binary expressions are evaluated in an
implementation-defined order:

AssignExpression, function arguments

It is an error to depend on order of evaluation when it is not
specified. For example, the following are illegal:

i = i++;"

Matheus.
Mar 29 2014
next sibling parent "ketmar" <ketmar ketmar.no-ip.org> writes:
 For example, the following are illegal:
 i = i++;"


so we have bug in compiler: it should not accept illegal 
expressions.
Mar 29 2014
prev sibling parent "Frustrated" <Who where.com> writes:
On Saturday, 29 March 2014 at 17:29:51 UTC, MattCoder wrote:

 On Saturday, 29 March 2014 at 16:31:48 UTC, Frustrated wrote:

 On Friday, 28 March 2014 at 19:57:13 UTC, MattCoder wrote:

 On Friday, 28 March 2014 at 19:35:22 UTC, Frustrated wrote:

 which means, i = is the previous value of j(before 
 increment).
 This should hold true when j is an alias for i.


 In this case: i = i++;  Means:

 "i" is incremented by one, but the value assigned in "i" is 
 the
 previous one. That's it.

 For me there's nothing wrong here.

 Matheus.



 One thing you are getting wrong/leaving out is that i++, even 
 in an assignment, increments i.


 I see your pain pal... so I took a look around:
 http://dlang.org/expression.html

 And according the page above:

 "Expressions

 C and C++ programmers will find the D expressions very familiar,
 with a few interesting additions.

 Expressions are used to compute values with a resulting type.
 These values can then be assigned, tested, or ignored.
 Expressions can also have side effects.

 Order Of Evaluation

 The following binary expressions are evaluated in strictly
 left-to-right order:
 [i]
 OrExpression, XorExpression, AndExpression, CmpExpression,
 ShiftExpression, AddExpression, CatExpression, MulExpression,
 PowExpression, CommaExpression, OrOrExpression, AndAndExpression
 [/i]

 The following binary expressions are evaluated in an
 implementation-defined order:

 AssignExpression, function arguments

 It is an error to depend on order of evaluation when it is not
 specified. For example, the following are illegal:

 i = i++;"

 Matheus.


My only point is that monarch was wrong ;) Trying to justify how 
dmd does it is like trying to prove your conclusion by stating it 
as a hypothesis. Regardless if the correct behavior is to 
increment i(which I think it should) or for it to be illegal, the 
wrong behavior is to leave i unchanged. Luckily I'd never write 
such a statement but I wonder if there are more complex cases 
that produce wrong behavior?
Mar 30 2014
prev sibling parent reply Justin Whear <justin economicmodeling.com> writes:
On Fri, 28 Mar 2014 19:35:20 +0000, Frustrated wrote:


 
 either way, all increment i, which actually never happens in D. As was
 pointed out, VS does it properly... D does it wrong. Accept it and stop
 trying to validate how D does it so you can say D is correct.
 
 Not only is it logically wrong, it is not consistent with previous
 interpretations of other compilers/languages nor with itself. It is
 wrong on all levels. Just because you believe in unicorns doesn't prove
 that they exist. All the evidence says you are wrong.
 


Nope, Frustrated is the one who is dead wrong.

test.c:
-----------------------------
#include <stdio.h>

int main()
{
	int i = 0;
	i = i++;
	printf("%i\n", i);
	return 0;
}
-----------------------------

$ gcc test.c && ./a.out
0

$ clang test.c && ./a.out
test.c:7:7: warning: multiple unsequenced modifications to 'i' [-
Wunsequenced]
        i = i++;
          ~  ^
1 warning generated.
0


Both gcc and clang agree that in C, assigning a post-increment to itself 
results in the original value.  At least Clang warns, which is good, but 
D is consistent with C's behavior on this point.

Justin
Mar 28 2014
next sibling parent Justin Whear <justin economicmodeling.com> writes:
Someone is sure to ask:

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.8/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 4.8.2-16' 
--with-bugurl=file:///usr/share/doc/gcc-4.8/README.Bugs --enable-
languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-
suffix=-4.8 --enable-shared --enable-linker-build-id --libexecdir=/usr/
lib --without-included-gettext --enable-threads=posix --with-gxx-include-
dir=/usr/include/c++/4.8 --libdir=/usr/lib --enable-nls --with-sysroot=/ 
--enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes 
--enable-gnu-unique-object --disable-libmudflap --enable-plugin --with-
system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-
cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.8-amd64/jre --enable-
java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.8-amd64 --
with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.8-amd64 --with-
arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --
enable-objc-gc --enable-multiarch --with-arch-32=i586 --with-abi=m64 --
with-multilib-list=m32,m64,mx32 --with-tune=generic --enable-
checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --
target=x86_64-linux-gnu
Thread model: posix
gcc version 4.8.2 (Debian 4.8.2-16)

$ clang -v
Debian clang version 3.3-16 (branches/release_33) (based on LLVM 3.3)
Target: x86_64-pc-linux-gnu
Thread model: posix
Mar 28 2014
prev sibling next sibling parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 03/28/2014 01:59 PM, Justin Whear wrote:


 $ clang test.c && ./a.out
 test.c:7:7: warning: multiple unsequenced modifications to 'i' [-
 Wunsequenced]
          i = i++;
            ~  ^
 1 warning generated.
 0


 Both gcc and clang agree that in C, assigning a post-increment to itself
 results in the original value.


The result of a test program cannot be proof of that. In fact, changing 
the value of a variable more than once between two sequence points is 
undefined behavior.


 At least Clang warns, which is good,


It is unfortunate that Clang does not use the word "undefined" there.


 but D is consistent with C's behavior on this point.


It is undefined behavior in both languages.

Ali
Mar 28 2014
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 03/28/2014 10:56 PM, Ali Çehreli wrote:

  >
  > Both gcc and clang agree that in C, assigning a post-increment to itself
  > results in the original value.

 The result of a test program cannot be proof of that. In fact, changing
 the value of a variable more than once between two sequence points is
 undefined behavior.

  > At least Clang warns, which is good,

 It is unfortunate that Clang does not use the word "undefined" there.

  > but D is consistent with C's behavior on this point.

 It is undefined behavior in both languages.


How do you reach that conclusion in the case of D?
Mar 28 2014
parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 03/28/2014 03:05 PM, Timon Gehr wrote:


 On 03/28/2014 10:56 PM, Ali Çehreli wrote:



 It is undefined behavior in both languages.


 How do you reach that conclusion in the case of D?


By trusting my ignorance on such an execution order in D. :)

Walter has been saying for a while that D will eventually make some 
execution orders standard (e.g. for function arguments). I would like to 
know if the situation has improved.

Ali
Mar 28 2014
next sibling parent Timon Gehr <timon.gehr gmx.ch> writes:
On 03/28/2014 11:12 PM, Ali Çehreli wrote:

 By trusting my ignorance on such an execution order in D. :)


Well, unspecified execution order by itself does not imply undefined 
behaviour.
Mar 28 2014
prev sibling next sibling parent "ketmar" <ketmar ketmar.no-ip.org> writes:
i hope there will be fixed evaluation order in D. spec writers 
(especially those who writing specs for C) tends to think 'too 
close to machine', saying 'unspecified' here and there to allow 
write compilers that can 'superoptimize' some code.

but what they nearly always forgot is that humans writing 
programs, not computers. so the language should be *first* clear 
to human writer and *only then* supports some optimizing tricks.

ideally there should be no 'undefined behavior' in specs at all.

if something is questionable from the point of a language user 
(not compiler writer), this should be fixed and documented. and 
declaring it 'undefined behavior' is not the proper way to fix 
it. ;-)
Mar 28 2014
prev sibling parent reply Iain Buclaw <ibuclaw gdcproject.org> writes:
On 28 March 2014 22:12, Ali Çehreli <acehreli yahoo.com> wrote:

 On 03/28/2014 03:05 PM, Timon Gehr wrote:


 On 03/28/2014 10:56 PM, Ali Çehreli wrote:



 It is undefined behavior in both languages.


 How do you reach that conclusion in the case of D?


 By trusting my ignorance on such an execution order in D. :)

 Walter has been saying for a while that D will eventually make some
 execution orders standard (e.g. for function arguments). I would like to
 know if the situation has improved.


Regardless of what anyone says.  So long as this bug is open (linked),
then order of evaluation can only be regarded as undefined. :-)

http://bugzilla.gdcproject.org/show_bug.cgi?id=8

Regards
Iain.
Mar 31 2014
parent Johannes Pfau <nospam example.com> writes:
Am Mon, 31 Mar 2014 18:00:07 +0100
schrieb Iain Buclaw <ibuclaw gdcproject.org>:


 On 28 March 2014 22:12, Ali =C3=87ehreli <acehreli yahoo.com> wrote:

 On 03/28/2014 03:05 PM, Timon Gehr wrote:


 On 03/28/2014 10:56 PM, Ali =C3=87ehreli wrote:



 It is undefined behavior in both languages.


 How do you reach that conclusion in the case of D?


 By trusting my ignorance on such an execution order in D. :)

 Walter has been saying for a while that D will eventually make some
 execution orders standard (e.g. for function arguments). I would
 like to know if the situation has improved.


=20
 Regardless of what anyone says.  So long as this bug is open (linked),
 then order of evaluation can only be regarded as undefined. :-)
=20
 http://bugzilla.gdcproject.org/show_bug.cgi?id=3D8
=20
 Regards
 Iain.
=20


I have a fix + test cases for that issue for GDC btw (function call
order, array op order, assignment order). But the frontend code changed
between 2.064 and 2.065 (visitor related) so I'll port it to 2.065
first, then merge it into dmd and probably only merge back into gdc
after the 2.065 frontend has been merged.
Mar 31 2014
prev sibling next sibling parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 03/28/2014 09:59 PM, Justin Whear wrote:

 On Fri, 28 Mar 2014 19:35:20 +0000, Frustrated wrote:


 either way, all increment i, which actually never happens in D. As was
 pointed out, VS does it properly... D does it wrong. Accept it and stop
 trying to validate how D does it so you can say D is correct.

 Not only is it logically wrong, it is not consistent with previous
 interpretations of other compilers/languages nor with itself. It is
 wrong on all levels. Just because you believe in unicorns doesn't prove
 that they exist. All the evidence says you are wrong.


 Nope, Frustrated is the one who is dead wrong.


Yup. Also, I don't get point of the hostility.

Executing the side effect of an expression after its value has already 
been computed and used certainly seems like the least reasonable choice.


 test.c:
 -----------------------------
 #include <stdio.h>

 int main()
 {
 	int i = 0;
 	i = i++;
 	printf("%i\n", i);
 	return 0;
 }
 -----------------------------

 $ gcc test.c && ./a.out
 0

 $ clang test.c && ./a.out
 test.c:7:7: warning: multiple unsequenced modifications to 'i' [-
 Wunsequenced]
          i = i++;
            ~  ^
 1 warning generated.
 0


 Both gcc and clang agree that in C, assigning a post-increment to itself
 results in the original value.  At least Clang warns, which is good, but
 D is consistent with C's behavior on this point.

 Justin


Not really. In C, the evaluation order of assignment and increment is 
actually unspecified and hence a conforming compiler can do it either way.

In D both the "spec" and DMD are confused about the issue of evaluation 
order. (A related fact is that the "spec" contradicts itself on the 
issue of function argument evaluation order.)

http://dlang.org/expression.html

"The following binary expressions are evaluated in an 
implementation-defined order:

AssignExpression, function arguments
...
The evaluation order of function arguments is defined to be left to right."


The example of order-dependent code the "spec" gives is actually the 
example we are talking about here. I'm not sure what "implementation 
defined evaluation order" is supposed to mean though. E.g. if it is just 
the order in which the left and right expression are evaluated before 
the assignment takes place, then the effect of i=i++; is not actually 
ambiguous, as 'i' does not have any side effect and is a fine fully 
evaluated lvalue.

IMHO the most reasonable way out is to just fix a canonical evaluation 
order (i.e. left-to-right) and then stick to that consistently. Under 
left-to-right evaluation order in particular, i=i++; should leave the 
value of i intact. (for int i).

Walter has mentioned on at least one occasion that evaluation should 
become left-to-right everywhere.
Mar 28 2014
parent Timon Gehr <timon.gehr gmx.ch> writes:
On 03/28/2014 10:58 PM, Timon Gehr wrote:


 Not really. In C, the evaluation order of assignment and increment is
 actually unspecified and hence a conforming compiler can do it either way.


(Or any other way. Ali is right that in C this is even undefined 
behaviour. Apparently in C++11 only one of the two behaviours can occur.)

http://stackoverflow.com/questions/4176328/undefined-behavior-and-sequence-points



 Mar 28 2014



prev sibling  parent reply "Frustrated" <Who where.com>  writes:


On Friday, 28 March 2014 at 20:59:34 UTC, Justin Whear wrote:

 On Fri, 28 Mar 2014 19:35:20 +0000, Frustrated wrote:


 
 either way, all increment i, which actually never happens in 
 D. As was
 pointed out, VS does it properly... D does it wrong. Accept it 
 and stop
 trying to validate how D does it so you can say D is correct.
 
 Not only is it logically wrong, it is not consistent with 
 previous
 interpretations of other compilers/languages nor with itself. 
 It is
 wrong on all levels. Just because you believe in unicorns 
 doesn't prove
 that they exist. All the evidence says you are wrong.
 


 Nope, Frustrated is the one who is dead wrong.

 test.c:
 -----------------------------
 #include <stdio.h>

 int main()
 {
 	int i = 0;
 	i = i++;
 	printf("%i\n", i);
 	return 0;
 }
 -----------------------------

 $ gcc test.c && ./a.out
 0

 $ clang test.c && ./a.out
 test.c:7:7: warning: multiple unsequenced modifications to 'i' 
 [-
 Wunsequenced]
         i = i++;
           ~  ^
 1 warning generated.
 0


 Both gcc and clang agree that in C, assigning a post-increment 
 to itself
 results in the original value.  At least Clang warns, which is 
 good, but
 D is consistent with C's behavior on this point.

 Justin



Oh yeah, this makes hell of a lot of sense. I'm the one who's 
wrong, yet monarch claims it his is correct and yet clang gives a 
warning. Great logic there! I'll be sure to mark every post you 
have as special so I can study your logic to become so smart.



 Mar 29 2014



  parent reply Justin Whear <justin economicmodeling.com>  writes:


On Sat, 29 Mar 2014 16:21:54 +0000, Frustrated wrote:
 

 
 Oh yeah, this makes hell of a lot of sense. I'm the one who's wrong, yet
 monarch claims it his is correct and yet clang gives a warning. Great
 logic there! I'll be sure to mark every post you have as special so I
 can study your logic to become so smart.


Trying to insult me isn't going to make you right, nor will an appeal to 
unicorns.  With a well-defined order-of-operations, this might be correct 
behavior;  in this case I suspect D is like C and that this is undefined 
behavior, thus I would support making this statement illegal or at least 
warning.  At any rate you are demonstrably incorrect about D differing 
from the behavior of all other C-family languages--so unless you want to 
go flame on the GCC and Clang mailing-lists, I suggest you dial it back.

Cheers,
Justin



 Mar 31 2014



  parent reply "monarch_dodra" <monarchdodra gmail.com>  writes:


On Monday, 31 March 2014 at 15:51:03 UTC, Justin Whear wrote:

 Trying to insult me isn't going to make you right, nor will an 
 appeal to
 unicorns.  With a well-defined order-of-operations, this might 
 be correct
 behavior;


Actually, regardless of order of operation, specs says you still 
get "i" un-incremented, since opEquals's lhs has no side effects 
anyways.

...The only way to have some other result, is if there was some 
sort of undefined behavior in the statement, at which point there 
is no more concept of "correct behavior"


 in this case I suspect D is like C and that this is undefined
 behavior


Yup. It's undefined behavior. I replied too fast and missed it.


 thus I would support making this statement illegal or at least
 warning.


The issue is that the legality of this can't be checked at the 
semantic phase.

It depends on *who* the left and right hand sides are. It's 
obvious in "i = i++;", but if you start mixing function calls, 
references and whatnot, the line becomes more blurry:

//----
int i;
auto p = &i;
*p = i++; //Legal? Illegal?
//----

So this means making it illegal is out of the question.

As for making it a warning, well, Walter is (AFAIK) pretty much 
against having any warnings at all, short of straight up 
deterministic semantic warnings "dangling else"/"deprecation".



 Mar 31 2014



  parent  Justin Whear <justin economicmodeling.com>  writes:


On Mon, 31 Mar 2014 16:07:25 +0000, monarch_dodra wrote:


 
 The issue is that the legality of this can't be checked at the semantic
 phase.
 
 It depends on *who* the left and right hand sides are. It's obvious in
 "i = i++;", but if you start mixing function calls, references and
 whatnot, the line becomes more blurry:
 
 //----
 int i;
 auto p = &i;
 *p = i++; //Legal? Illegal?
 //----
 
 So this means making it illegal is out of the question.
 
 As for making it a warning, well, Walter is (AFAIK) pretty much against
 having any warnings at all, short of straight up deterministic semantic
 warnings "dangling else"/"deprecation".


Yeah, I understand that detecting anything more than the very simplest 
cases may be impossible--I'm thinking of an error similar to the existing 
"var has no effect in expression" which has helped me catch typos and 
simple mistakes.

Justin



 Mar 31 2014



prev sibling next sibling parent  "Meta" <jared771 gmail.com>  writes:


 It is a bug. It might be correct in monarch's world but it is 
 not logical. I think monarch is just trying to justify the way 
 D does it, regardless if D is D is wrong or not.


You mean how *every* C-derived language does it?



 Mar 28 2014



prev sibling next sibling parent  Timon Gehr <timon.gehr gmx.ch>  writes:


On 03/28/2014 07:04 PM, Frustrated wrote:

 I imagine what is going on is that D is creating a temp variable,
 assigning it to the local variable, then incrementing the temp
 variable(since++ increments "after" the assignment).


int tmp = i;
++i;
i = tmp;



 Mar 28 2014



prev sibling  parent reply "John Colvin" <john.loughran.colvin gmail.com>  writes:


On Friday, 28 March 2014 at 18:04:41 UTC, Frustrated wrote:

 On Friday, 28 March 2014 at 16:54:49 UTC, Benjamin Thaut wrote:

 I had a bug which came down to the following line today:

 m_takeIndex = m_takeIndex++;

 Actually this line does nothing. m_takeIndex remains at 
 exactly the same value as before (tested with dmd 2.065).

 Can someone please explain why? And would be suitable to warn 
 about this, if it is correct behaviour?

 Kind Regards
 Benjamin Thaut


 This should be invalid.

 m_takeIndex++;

 actually increments the variable.

 m_takeIndex = m_takeIndex++; should do exactly the same.


Nope. Nope nope nope. That's not what postfix increment is. 
That's why prefix increment exists.

Yeah, it might be an odd design (dates back to poor optimising 
compilers and fast INC/DEC instructions apparently), but that's 
how it is in C and how it is in D, by inheritance.



 Mar 31 2014



  parent reply "Dominikus Dittes Scherkl"  writes:


On Monday, 31 March 2014 at 15:56:29 UTC, John Colvin wrote:

 m_takeIndex = m_takeIndex++; should do exactly the same.


 Nope. Nope nope nope. That's not what postfix increment is. 
 That's why prefix increment exists.

 Yeah, it might be an odd design (dates back to poor optimising 
 compilers and fast INC/DEC instructions apparently), but that's 
 how it is in C and how it is in D, by inheritance.


I just don't see, why one would expect the assignment could come 
first?!?
The post-increment has a much, much higher priority, so I would 
think it is obvious that first the value is incremented and 
afterwards the old value is assigned to it.
And of course, the old, non-incremented value is assigned. This 
is the essence od POST incrementing - the return value of this 
operator is defined to be the old value, that's the whole point 
why this operator exists. (btw. I would _never_ever_ write a line 
like "i++;" That's garbage. It should always be "++i;"
Yes, the optimizer will get rid of the temporary variable, but it 
is absolutely unintuitive (if I read something like that my first 
thought is: what is the
old value used for? Why using a temporary variable?)

But I fully agree that such a form of complicated NOP ("i = 
i++;") should be forbidden or at least warned about.



 Mar 31 2014



  parent reply "monarch_dodra" <monarchdodra gmail.com>  writes:


On Monday, 31 March 2014 at 16:18:13 UTC, Dominikus Dittes 
Scherkl wrote:

 (btw. I would _never_ever_ write a line like "i++;" That's 
 garbage. It should always be "++i;"
 Yes, the optimizer will get rid of the temporary variable, but 
 it is absolutely unintuitive (if I read something like that my 
 first thought is: what is the
 old value used for? Why using a temporary variable?)


Using "i++" can make a lot of sense, especially when you are 
iterating indexes. Very often, you want the data *at* i, and then 
you want to increment i. It makes sense to write "p[i++]", and 
avoids writing an extra instruction.

A "famous" example is the strcpy (example) implementation, which 
is just:
while(*dest++ = *src++);

You wouldn't write that with ++i.


 The post-increment has a much, much higher priority, so I would 
 think it is obvious that first the value is incremented and 
 afterwards the old value is assigned to it.
 And of course, the old, non-incremented value is assigned. This 
 is the essence od POST incrementing - the return value of this 
 operator is defined to be the old value, that's the whole point 
 why this operator exists.


The issue at hand is that most compilers don't do "verbatim" what 
you type. They are actually allowed to do whatever the heck they 
want, provided the *observable* behavior is what you typed. This 
is good, because it allows them to take shorcuts, or "optimize".

The issue is, when they take these shortcuts, they assume 
(rightfully) that your code conforms to spec. If not, then 
"undefined behavior" is what happens.

i = i++;

This does not conform to spec because "i" is written to and read 
to, all in between two sequence points. This is illegal, and 
produces undefined behavior.

Here is how it can happen:
=> Compiler reads "i =". Thinks "OK, I need to assign something 
to i".
=> Then reads "operator++(i)". Thinks, "OK, I need to assign the 
result of that to i".
=> Then thinks "I know what the result of i++ is, and i++ isn't 
legally allowed to modify i, since I just read it"
=> Compiler "assigns i to i" (value is unchanged)
=> Compiler executes i++ (value is incremented).

Here is another fun story. I once encountered this code.
i = ++i;
With the right compiler options, the result was that 'i' was 
incremented... twice!

Go! Go! Undefined behavior!



 Mar 31 2014



  parent  "Dominikus Dittes Scherkl"  writes:


On Monday, 31 March 2014 at 18:16:32 UTC, monarch_dodra wrote:

 On Monday, 31 March 2014 at 16:18:13 UTC, Dominikus Dittes 
 Scherkl wrote:

 (btw. I would _never_ever_ write a line like "i++;" That's 
 garbage. It should always be "++i;"





 Using "i++" can make a lot of sense, especially when you are 
 iterating indexes.


Of course the post-increment is often very usefull - iff you use
it's return value. Therefore the semicolon.
i++;
as a whole line is very bad style, I think.


 i = i++;

 This does not conform to spec because "i" is written to and 
 read to, all in between two sequence points. This is illegal, 
 and produces undefined behavior.


Why?
If the operators had the same precedence, ok.
But ++ has to come first (no matter if post or pre).
something like
j = --i++;
yeah, that is undefined (pre- and post having same priority).

But does the spec really say a value may not at the same time be 
read from and written to? Then i=i; would also be illegal. And 
how about i += i; ? I think this is allowed and useful, ne?
What about i = ++i; (ok does nothing diffrent from ++i; but is 
still ok and not a NOP)?

i = i++; is unambigous if the compiler doesn't optimize it. And 
if it optimizes, it should replace it by NOP.
But it is likely that this is a typo or a logic-bug by the user, 
so the compiler should instead warn or even forbid it.

As a general rule in one expression one variable should not be
allowed to be modified more than once.

e.g.
i++ = i;
j = i++ / i++;
i += i++;
etc. should all be forbidden.


 Go! Go! Undefined behavior!





 Mar 31 2014