2

In reference to this question.

I have tried this same program many times and I have seen others (group of friends ) using the same logic for swapping but none of them ever found wrong output. I want to ask that is there any chance of getting wrong output due to sequence point.

Community
  • 1
  • 1
Lovely
  • 43
  • 1
  • 6
  • 7
    Why not simply put it on another line? Makes it easier to read for programmers as well. Eg. I don't even know what to expect as result. – Caramiriel Feb 11 '16 at 13:22
  • 5
    This is horrible coding. Why would you ever want to do this?? – r3mainer Feb 11 '16 at 13:23
  • 1
    This is actually bad coding style. :-( *edit* other were faster :) – HelloWorld Feb 11 '16 at 13:23
  • @Caramiriel you can edit this question – Lovely Feb 11 '16 at 13:24
  • If you used a good compiler or had the warnings turned up it would have told you: http://coliru.stacked-crooked.com/a/f7ba7ea62ca5732d – NathanOliver Feb 11 '16 at 13:24
  • You need to look at operator precedence in C++. –  Feb 11 '16 at 13:25
  • @squeamishossifrage just in case if we are asked to swap two variables using one expression without using any function – Lovely Feb 11 '16 at 13:25
  • 1
    @LovelyUpadhyay no need to, because I want to make clear that the code can be unreadable for programmers. The question is perfectly valid. – Caramiriel Feb 11 '16 at 13:25
  • @NathanOliver yes, I saw that warning on GCC 5.1 compiler (this is also given in the question I reffered) – Lovely Feb 11 '16 at 13:26
  • 3
    "in case if we are asked to swap two variables using one expression without using any function": except for interview questions, who the hell asks for such a thing? It just doesn't make sense. It is unreadable, it's dangerous as it is UB, and it isn't even faster than the normal swap approach. – Fabio says Reinstate Monica Feb 11 '16 at 13:32
  • 1
    You'll get wrong result with [vc](http://rextester.com/UJBM50913). – songyuanyao Feb 11 '16 at 13:32
  • 2
    BTW if you want to swap items you should use [`std::swap`](http://en.cppreference.com/w/cpp/algorithm/swap). More than likely it will be faster then the arithmetic operations and it is perfectly clear what you are doing. – NathanOliver Feb 11 '16 at 13:34
  • @NathanOliver: Faster or not, it clearly conveys intent. That's reason enough to use `std::swap`. – IInspectable Feb 11 '16 at 13:37
  • @songyuanyao: How can an expression, that exhibits UB produce the *"wrong result"*? The behavior is undefined, any result is equally valid. – IInspectable Feb 11 '16 at 13:42
  • Re: " swap two variables using one expression" - that's a **horrible** assignment, unless its purpose is to teach bad coding habits. I haven't yet seen an attempt at this that works right for all cases. Usually they fall down when the code tries to swap a variable with itself. – Pete Becker Feb 11 '16 at 15:10

6 Answers6

8

C++11 doesn't have sequence points anymore, but yes, the line is undefined behavior because the modification of b is not sequenced relative to its read.

This means that anything can happen; in general, though, the main problem is that compilers might reorder the exact sequence of events.

Sebastian Redl
  • 69,373
  • 8
  • 123
  • 157
  • 1
    Good answer. Btw, [this](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2239.html) paper prooves your statement. – HelloWorld Feb 11 '16 at 13:28
  • why this `a ^= b, b ^= a, a ^= b;` does not give any sequence point warning? – Lovely Feb 11 '16 at 14:11
  • "A pair of expressions separated by a comma is evaluated left-to-right; ..." - N3337, 5.18 – Caramiriel Feb 11 '16 at 14:17
  • @LovelyUpadhyay: The [built-in comma operator](http://en.cppreference.com/w/cpp/language/operator_other#Built-in_comma_operator) guarantees sequencing, so there is no warning. If obfuscation is the goal you could go with `a ^= b, a ^= b ^= a;` instead. – IInspectable Feb 11 '16 at 21:38
3

Yes, as far as I can tell, this is undefined behavior. The semicolon here is the only sequence point, so it is undefined whether the assignment takes place before or after the same variable gets used.

Now, if all your group of friends are using the same compiler and the same platform, which seems likely, they're all going to see the same results, with the same compiler, so this is not surprising. That's the answer to that part of the question.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
1

Basically, yes. It may give wrong result, because in this line B is both written and read, and it is unspecified what will happen first.

Most probably you have tried it many times but you have used the same compiler, right? In such case it's very unlikely for you to observe different results. For a given the same bit of code, compilers usually produce a stable the same result.

To see a difference, you may need to change the compiler, or at least change some options like more or less aggressive optimization.

The problem with this expression is that, theoretically, it may be compiled as:

assign b  <- a
a = a + b - a     // but now, B is already equal a

or

assign temp1 <- a
assign temp2 <- b
assign b  <- a
a = temp1 + temp2 - a   // here values are preserved
quetzalcoatl
  • 32,194
  • 8
  • 68
  • 107
1

Yes, this is undefined behavior and it will give following warning

$ g++ -Wall -o test test.cpp test.cpp: In function ‘int main()’: test.cpp:11:21: warning: operation on ‘b’ may be undefined [-Wsequence-point]

Neeraj Jha
  • 123
  • 1
  • 8
1

If you use the above "trick" instead of a standard swap, with Visual Studio, you will have an unpleasant surprise. The side effects of evaluations are still here.

zdf
  • 4,382
  • 3
  • 18
  • 29
0

The C standard (1999 ed.) says in section 6.5 clause 2:

Between the previous and next sequence point an object shall have its stored value modified at most once by the evaluation of an expression. Furthermore, the prior value shall be read only to determine the value to be stored.

So, yes, this code violates the sequence point rules (b is read from but not to determine the new value of b). C++ inherits this from C.

Alexey Frunze
  • 61,140
  • 12
  • 83
  • 180
  • Do you know where it says this rules is inherited? – NathanOliver Feb 11 '16 at 13:36
  • @NathanOliver The C++ standard from 2003 has either exactly the same or very similar definition and treatment of sequence points and says the same as I quoted above in section 5 clause 4. – Alexey Frunze Feb 11 '16 at 13:43
  • 1
    You are right on one point: C++ inherits from C and as it is UB in C it is likely to be in C++. But C and C++ are now different languages and there are corner cases where they behave differently. So using C standard to say that a C++ expression is invalid is clearly not enough. You would need a reference form C++ standard explicitely saying that this part of C standard must be respected in a valid C++ program (or directly the transposition from C++ standard) – Serge Ballesta Feb 11 '16 at 13:45
  • Yes but that does not mean C++ inherits from C. Yes C++ was made to be mostly backwards compatible and most C programs could be compiled as C++ but with the divergence of the standards in recent years I would hesitate to have a blanket statement that C++ inherits from C anymore – NathanOliver Feb 11 '16 at 13:46
  • @SergeBallesta I'm well aware of C++ being very different from C and having subtle differences in what would seem to be the same thing at first glance. I provided references to the relevant text in both C and C++ standards. – Alexey Frunze Feb 11 '16 at 13:49
  • 1
    @AlexeyFrunze You should also consider that sequence points were used until C++03; [in C++11 sequence points have been superseded by the concept of sequencing](http://stackoverflow.com/a/4183735/3982001). So I'm not sure referring to sequence points in C is a good idea to talk about what happens in C++. – Fabio says Reinstate Monica Feb 11 '16 at 13:51
  • @FabioTurati But you're not saying sequence points have not existed in C++, right? And the OP isn't specifying which C++ he's talking about, right? And the expression still results in undefined behavior even in C++11, right? And then your point is...? – Alexey Frunze Feb 11 '16 at 14:23