3

The if statement in the following sample comes from an old project I am trying to build again. I am sorry this is not a verifiable sample in the sense that it does not reproduce the error, it compiles fine by itself.

enum S { };

struct R {
    S type, state;
    double height;
};

int main ()
{
    int rows;
    S alive, rc;
    double h;
    R *r, *n;

    // BEGIN
    if ( (((r = n-1)   ->type & rc) && h > r->height) ||
         (((r = n+1)   ->type & rc) && h > r->height) ||
         (((r = n-rows)->type & rc) && h > r->height) ||
         (((r = n+rows)->type & rc) && h > r->height) )
    {
        n->state = alive;
    }
    // END
}

However, within the project, with the same compilation (clang 3.3 with no options except include paths), it gives

warning: multiple unsequenced modifications to 'r' [-Wunsequenced]

(pointing to sub-expression r = n - 1). The if statement (marked between BEGIN-END) is identical within the project, only the types of the variables may be different. The types defined here are only crude approximations, but I think they are not really relevant to the warning. I have been able to simplify the expression down to

if( (r = n) || (r = n) ) //...

while still reproducing the warning within the project.

My understanding is that there is a sequence point after the evaluation of the first operand of operators &&, || (when not overloaded). So I cannot explain the warning.

If there are any ideas concerning the form of the expression regardless of types, that would help.

EDIT

Using Vaughn Cato's comments, I have also found that

if( ((r = n-1) ->type) || ((r = n+1) ->type) )

produces the warning, while

if( bool((r = n-1) ->type) || bool((r = n+1) ->type) )

does not. S is in fact a class that mimics an enumeration, and has a custom underlying class. It has overloaded bitwise operators &, | and ! for masking, and implicit conversion operator to the underlying type, which in the particular case is size_t. I don't know how much this helps, but I am sorry for not disclosing more complete information right from the start.

iavr
  • 7,547
  • 1
  • 18
  • 53
  • Could the types involved have user-defined operator `&&` and `||`? Only the built-in operators have the sequence guarantee. – Vaughn Cato Apr 02 '14 at 13:13
  • Only `operator&` is overloaded, but the warning remains even in the simplified expression without `&`. Besides, `r,n` are pointers, so I don't think I could overload anything else. – iavr Apr 02 '14 at 13:14
  • +1: This is a deep question. I've thrown an answer into the ring but wait with interest for a more detailed response. – Bathsheba Apr 02 '14 at 13:21
  • Is the type of `r` in the project also a raw pointer? – Vaughn Cato Apr 02 '14 at 13:22
  • Note that nowadays the concept of *secuence points* has been deprecated in favor of a well defined standard multi-threaded memory model. – Manu343726 Apr 02 '14 at 13:25
  • @VaughnCato Yes, `r` is a raw pointer. `R` is a plain `struct`. `S` is not really an `enum` but rather a wrapper `enumeration` class that was then an attempt to simulate having my own underlying type. – iavr Apr 02 '14 at 13:27
  • @Manu343726 Now that you mention the memory model, `R` actually contained `union`'s (to information that was used in mutually exclusive time instances), but `type,state,height` occupied distinct positions. Only `height` shared the same position with another member in a `union`. – iavr Apr 02 '14 at 13:32
  • In your project, try using `if( bool(r = n) || bool(r = n) )` and see if you get the same warning. – Vaughn Cato Apr 02 '14 at 13:39
  • @VaughnCato Interesting idea... and the warning is **gone** :-) – iavr Apr 02 '14 at 13:46
  • I'm thinking there must be an overloaded `operator ||` in your environment that is being invoked somehow, and casting to bool is making it avoid using the overload. – Vaughn Cato Apr 02 '14 at 13:57
  • @VaughnCato Please see my edit, it may help. – iavr Apr 02 '14 at 14:16
  • @ShafikYaghmour No, because I haven't looked into it yet. I suspect the overloaded operators `|` and `&` but I cannot be sure. When I try again, I may get away with the `bool()` trick, replace `S` with a real `enum` with custom underlying type, or anything else that may come up. I'm happy there's already a workaround because this is a really "smart" expression I wouldn't like to change. – iavr Apr 09 '14 at 21:58

2 Answers2

2

See N3797 1.9/15

Except where noted, evaluations of operands of individual operators and of subexpressions of individual expressions are unsequenced.

5.14/2: (the && operator)
5.15/2: (the || operator)

If the second expression is evaluated, every value computation and side effect associated with the first expression is sequenced before every value computation and side effect associated with the second expression.

The reference to 'side effect' means that the value of r is properly sequenced.

My reading is that this warning would be incorrect. The expression is properly sequenced (excluding any weird effects from code not shown here).

david.pfx
  • 10,520
  • 3
  • 30
  • 63
1

There's a common confusion here. Just because || and && are sequencing points does not imply that the runtime evaluates r in the order you think.

It could evaluate r = n - 1, r = n + 1 etc. before any of the if expressions are evaluated in any order it fancies; i.e. is unsequenced.

That is what the compiler warning is highlighting to you.

By the way, even languages that purport to have better defined sequencing (e.g. Java) will suffer from the effect I'm drawing your attention to here!

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • Sorry, I don't quite get this. Besides sequence points, there's required [short-circuiting and evaluation order](http://stackoverflow.com/a/628554/2644390), right? – iavr Apr 02 '14 at 13:18
  • Yup indeed there is **but** the runtime could still attempt to pre-compute all the `r = ` assignment expressions. There are some *real* experts on this site who will be able to answer this in a lot of detail: let's see what we get. – Bathsheba Apr 02 '14 at 13:19
  • You say the runtime may compute quantities in different order than the language specifies? This is so hard to follow... – iavr Apr 02 '14 at 13:22
  • Your assignments to `r` *can* be computed before the `if` evaluation even starts. And they are unsequenced as I say in my answer. – Bathsheba Apr 02 '14 at 13:26