Bug 22248

Summary: Incorrect work with multiple assigment.
Product: gcc Reporter: Bogdan Yakovenko <algorithmus>
Component: c++Assignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED DUPLICATE    
Severity: normal CC: gcc-bugs
Priority: P2    
Version: 4.0.1   
Target Milestone: ---   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed:

Description Bogdan Yakovenko 2005-06-30 13:03:22 UTC
hi!

I have a problem with multiply assigment at 
i386-portbld-freebsd5.4
gcc version 4.0.1 20050609 (prerelease) [FreeBSD]

and the same problem in DJGPP with gcc 4.0.0

I've compiled my cpp file with this command line:
g++40 new.cpp

---new.cpp----
#include<cstdio>

#define swap(a,b) a^=b^=a^=b

const int N = 2;
int a[N];

int main()
{
        for(int i=0;i<N;i++) a[i] = i;
        for(int i=0;i<N/2;i++) swap(a[i],a[N-i-1]);
        for(int i=0;i<N;i++) printf("%d ",a[i]);
        printf("\n");
        return 0;
}
-------

This program simply makes 2-element array: a[0] = 0 a[1] = 1;
After that it uses macros swap(a[0],a[1]) to swap these elements.

After swaping program makes a[0] = 0 a[1] = 0 that's not correct.

Actually, if you compile this program with -O2 key: g++40 -O2 new.cpp
program will work fine and make a[0] = 1 a[1] = 0;

The same situation(correct work) will be if you use older gcc 3.x compiler.

I've done several experements and I guess that something wrong causes after last
assigment.
Comment 1 Gabriel Dos Reis 2005-06-30 13:19:04 UTC
Subject: Re:  New: Incorrect work with multiple assigment.

"algorithmus at gmail dot com" <gcc-bugzilla@gcc.gnu.org> writes:

| ---new.cpp----
| #include<cstdio>
| 
| #define swap(a,b) a^=b^=a^=b

You're modifying objects more than once without intervening sequence
points.  All bets are off. There is a standard utility std::swap() from
<utility> that works all the time.
PR invalid.

-- Gaby
Comment 2 Andrew Pinski 2005-06-30 13:26:39 UTC
There is no sequence points between the assignments of a[i] so the loading of a[i] can be evaluated  in 
any order. 
You want to define swap as:
#define swap(a,b) do{ a^=b;b^=a;a^=b;}while(0)


This is a dup of bug 11751.

*** This bug has been marked as a duplicate of 11751 ***
Comment 3 Bogdan Yakovenko 2005-06-30 13:35:44 UTC
no. I guess there is a problem.
just try this progam:

------------
#include<cstdio>

#define swap(a,b) (a^=(b^=(a^=b)))

const int N = 2;
int a[N];

int main()
{
        for(int i=0;i<N;i++) a[i] = i;
        for(int i=0;i<N/2;i++) swap(a[i],a[N-i-1]);
        for(int i=0;i<N;i++) printf("%d ",a[i]);
        printf("\n");
        return 0;
}
--------

the same result... 0 0
Comment 4 Andrew Pinski 2005-06-30 13:37:48 UTC
Parentheses are not seqeunce points.  The comma operator is though.

*** This bug has been marked as a duplicate of 11751 ***
Comment 5 Giovanni Bajo 2005-06-30 17:58:04 UTC
Also you could simply use std::swap as Gaby suggested.
Comment 6 Bogdan Yakovenko 2005-06-30 18:21:24 UTC
thanks for your suggestions. I usually use STL. But I held training contest for 
Ukrainian team before International Olympiad in Informatics and one guy's used 
this swap macros that doesn't work at my compiler but works fine with his. 

I guess, it'll be useful if the developers add some warning message for similar-
to-this cases.
Comment 7 Giovanni Bajo 2005-06-30 18:27:19 UTC
It's there: -Wsequence-point, which is also enabled by -Wall.
Comment 8 Andrew Pinski 2005-06-30 18:31:32 UTC
(In reply to comment #7)
> It's there: -Wsequence-point, which is also enabled by -Wall.
But it will not warn about this testcase as we have array[index] but that is filed as PR 16202.
Comment 9 Gabriel Dos Reis 2005-06-30 20:57:45 UTC
Subject: Re:  Incorrect work with multiple assigment.

"algorithmus at gmail dot com" <gcc-bugzilla@gcc.gnu.org> writes:

| I guess, it'll be useful if the developers add some warning message
| for similar-to-this cases.

Did you try -Wsequence-points (or similar)?

-- Gaby
Comment 10 Bogdan Yakovenko 2005-07-01 05:04:07 UTC
(In reply to comment #9)
> 
> Did you try -Wsequence-points (or similar)?

yeah. both -Wall and -Wsequence-points return nothing.
Comment 11 Gabriel Dos Reis 2005-07-01 05:10:52 UTC
Subject: Re:  Incorrect work with multiple assigment.

"algorithmus at gmail dot com" <gcc-bugzilla@gcc.gnu.org> writes:

| (In reply to comment #9)
| > 
| > Did you try -Wsequence-points (or similar)?
| 
| yeah. both -Wall and -Wsequence-points return nothing.

then reclassify the bug as missed warning, (not incorrect code).

-- Gaby
Comment 12 Andrew Pinski 2005-07-01 05:13:38 UTC
(In reply to comment #11)
> then reclassify the bug as missed warning, (not incorrect code).
Did you read my comment #8, which I referenced the bug about the warning:
> But it will not warn about this testcase as we have array[index] but that is filed as PR 16202.
Comment 13 Gabriel Dos Reis 2005-07-01 05:23:45 UTC
Subject: Re:  Incorrect work with multiple assigment.

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| (In reply to comment #11)
| > then reclassify the bug as missed warning, (not incorrect code).
| Did you read my comment #8,

I read it.  I was responding to the bug submitter.

Do assume, people are not blind.

| which I referenced the bug about the warning:
| > But it will not warn about this testcase as we have array[index] but that is filed as PR 16202.

I don't think you're explaining that we cannot warn.  Are you?

-- Gaby
Comment 14 Bogdan Yakovenko 2005-07-01 12:42:21 UTC
> | > But it will not warn about this testcase as we have array[index] but that 
is filed as PR 16202.
> I don't think you're explaining that we cannot warn.  Are you?

I can't understand what Andrew means. I agree that we should reclassify this 
bug as missing warning. 

Because sometimes programmers could get into this problem.
Comment 15 Andrew Pinski 2005-07-01 14:29:10 UTC
(In reply to comment #14)
> > | > But it will not warn about this testcase as we have array[index] but that 
> is filed as PR 16202.
> > I don't think you're explaining that we cannot warn.  Are you?
I am explaining why we currently don't warn and that the diagnostic problem is already filed as PR 
16202.

*** This bug has been marked as a duplicate of 11751 ***