When compiling the following code with g++ (Ubuntu 8.3.0-6ubuntu1~18.04.1) 8.3.0, flags -std=gnu++17 -W{all,extra,error} int main() { int i = 0; int j = i++ << i++; } The following warning/error is produced: test.cc:3:14: error: operation on ‘i’ may be undefined [-Werror=sequence-point] int j = i++ << i++; ~^~ Whereas C++17 in Shift operators [expr.shift] requires that in E1 << E2 the expression E1 is sequenced before the expression E2.
gcc-9.1 produces the same warning.
So shall we emit no warnings with -std=c++17 -Wsequence-point on: struct S { int a[10]; }; void foo (int i, int (&x)[10][10], int y[10], S z[10], S *w[10]) { int b = x[i++][i++]; int c = i++ << i++; int d = i++ >> i++; int e = i++ && i++; int f = i++ ? i++ : i++; int g = (i++, i++); int h = z[i++].a[i++]; int j = w[i++]->a[i++]; y[i++] = y[i++]; } and warnings on 6 lines for -std=c++14? Right now we emit warnings on those 6 lines in both standard modes. clang 7 does the same, clang 9 emits them just on <<, >> and assignment. + add a testcase for .* and ->* too, what else?
--- gcc/c-family/c-common.c.jj 2019-07-30 08:27:49.987555303 +0200 +++ gcc/c-family/c-common.c 2019-08-10 18:13:20.821949299 +0200 @@ -1889,6 +1889,7 @@ verify_tree (tree x, struct tlist **pbef case COMPOUND_EXPR: case TRUTH_ANDIF_EXPR: case TRUTH_ORIF_EXPR: + sequenced_binary: tmp_before = tmp_nosp = tmp_list2 = tmp_list3 = 0; verify_tree (TREE_OPERAND (x, 0), &tmp_before, &tmp_nosp, NULL_TREE); warn_for_collisions (tmp_nosp); @@ -2031,8 +2032,18 @@ verify_tree (tree x, struct tlist **pbef x = TREE_OPERAND (x, 0); goto restart; } - gcc_fallthrough (); + goto do_default; + + case LSHIFT_EXPR: + case RSHIFT_EXPR: + case COMPONENT_REF: + case ARRAY_REF: + if (cxx_dialect >= cxx17) + goto sequenced_binary; + goto do_default; + default: + do_default: /* For other expressions, simply recurse on their operands. Manual tail recursion for unary expressions. Other non-expressions need not be processed. */ fixes most of this, except for the assignment operator case, plus the .*/-> case isn't addressed either.
Testcase also with PMF: struct S { int a[10]; void bar (); void baz (); }; typedef void (S::*pmf) (); void foo (int i, int x[10][10], int y[10], struct S z[10], struct S *w[10], pmf u[10]) { int b = x[i++][i++]; int c = i++ << i++; int d = i++ >> i++; int e = i++ && i++; int f = i++ ? i++ : i++; int g = (i++, i++); int h = z[i++].a[i++]; int j = w[i++]->a[i++]; (z[i++].*u[i++]) (); (w[i++]->*u[i++]) (); y[i++] = y[i++]; }
I think this is P0145R3 Refining Expression Evaluation Order for Idiomatic C++ http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0145r3.pdf which says 4. A SOLUTION In summary, the following expressions are evaluated in the order a, then b, then c, then d: 1. a.b 2. a->b 3. a->*b 4. a(b1, b2, b3) 5. b @=a 6. a[b] 7. a << b 8. a >> b
Created attachment 46702 [details] gcc10-pr91415-1.patch Let's handle this incrementally, from the easiest cases to the hardest.
On top of the above, I've tried: --- gcc/c-family/c-common.c.jj 2019-08-12 09:45:54.463491950 +0200 +++ gcc/c-family/c-common.c 2019-08-12 12:01:32.783135654 +0200 @@ -1933,16 +1934,19 @@ verify_tree (tree x, struct tlist **pbef tmp_before = tmp_nosp = tmp_list3 = 0; verify_tree (TREE_OPERAND (x, 1), &tmp_before, &tmp_nosp, NULL_TREE); verify_tree (TREE_OPERAND (x, 0), &tmp_list3, &tmp_list3, x); - /* Expressions inside the LHS are not ordered wrt. the sequence points - in the RHS. Example: + /* In C and C++ before 17 expressions inside the LHS are not ordered + wrt. the sequence points in the RHS. Example: *a = (a++, 2) Despite the fact that the modification of "a" is in the before_sp list (tmp_before), it conflicts with the use of "a" in the LHS. We can handle this by adding the contents of tmp_list3 to those of tmp_before, and redoing the collision warnings for that list. */ - add_tlist (&tmp_before, tmp_list3, x, 1); - warn_for_collisions (tmp_before); + if (cxx_dialect < cxx17) + { + add_tlist (&tmp_before, tmp_list3, x, 1); + warn_for_collisions (tmp_before); + } /* Exclude the LHS itself here; we first have to merge it into the tmp_nosp list. This is done to avoid warning for "a = a"; if we didn't exclude the LHS, we'd get it twice, once as a read and once --- gcc/testsuite/g++.dg/warn/sequence-pt-1.C.jj 2011-07-11 10:39:37.000000000 +0200 +++ gcc/testsuite/g++.dg/warn/sequence-pt-1.C 2019-08-12 12:22:42.033011124 +0200 @@ -35,7 +35,7 @@ foo (int a, int b, int n, int p, int *pt *ptr++ = (size_t)ptr++; /* { dg-warning "undefined" "sequence point warning" } */ sptr->a = sptr->a++; /* { dg-warning "undefined" "sequence point warning" } */ sptr->a = (size_t)(sptr++); /* { dg-warning "undefined" "sequence point warning" } */ - *ptr++ = fn (*ptr); /* { dg-warning "undefined" "sequence point warning" } */ + *ptr++ = fn (*ptr); /* { dg-warning "undefined" "sequence point warning" { target c++14_down } } */ a = b = a++; /* { dg-warning "undefined" "sequence point warning" } */ b = a = --b; /* { dg-warning "undefined" "sequence point warning" } */ a = 1 + (a = 1); /* { dg-warning "undefined" "sequence point warning" } */ @@ -47,13 +47,13 @@ foo (int a, int b, int n, int p, int *pt a = (*fnp[b++]) (b++); /* { dg-warning "undefined" "sequence point warning" } */ a = (*fnp[b]) (b++); /* { dg-warning "undefined" "sequence point warning" } */ a = (*fnp[b++]) (b); /* { dg-warning "undefined" "sequence point warning" } */ - *ap = fnc (ap++); /* { dg-warning "undefined" "sequence point warning" } */ + *ap = fnc (ap++); /* { dg-warning "undefined" "sequence point warning" { target c++14_down } } */ (a += b) + (a += n); /* { dg-warning "undefined" "sequence point warning" } */ a = (b, b++) + (b++, b); /* { dg-warning "undefined" "sequence point warning" } */ ap[a++] += a; /* { dg-warning "undefined" "sequence point warning" } */ ap[a+=1] += a; /* { dg-warning "undefined" "sequence point warning" } */ - ap[a++] += a++; /* { dg-warning "undefined" "sequence point warning" } */ - ap[a+=1] += a++; /* { dg-warning "undefined" "sequence point warning" } */ + ap[a++] += a++; /* { dg-warning "undefined" "sequence point warning" { target c++14_down } } */ + ap[a+=1] += a++; /* { dg-warning "undefined" "sequence point warning" { target c++14_down } } */ a = a++, b = a; /* { dg-warning "undefined" "sequence point warning" } */ b = a, a = a++; /* { dg-warning "undefined" "sequence point warning" } */ a = (b++ ? n : a) + b; /* { dg-warning "undefined" "sequence point warning" } */ to handle the assignment operator, but while that handles some cases as shown in the testsuite that IMHO are now well defined in C++17, it doesn't handle the case from the testcase: void foo (int i, int y[10]) { y[i++] = y[i++]; } So I probably don't understand the code well enough :(. And the PMF case is something to be resolved later too.
Oh, and need to look also at CALL_EXPR.
Author: jakub Date: Tue Aug 27 12:37:30 2019 New Revision: 274952 URL: https://gcc.gnu.org/viewcvs?rev=274952&root=gcc&view=rev Log: PR c++/91415 * c-common.c (verify_tree): For LSHIFT_EXPR, RSHIFT_EXPR, COMPONENT_REF and ARRAY_REF in cxx_dialect >= cxx17 mode handle it like COMPOUND_EXPR rather than normal expression. * g++.dg/warn/sequence-pt-4.C: New test. Added: trunk/gcc/testsuite/g++.dg/warn/sequence-pt-4.C Modified: trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-common.c trunk/gcc/testsuite/ChangeLog
Partially fixed, needs more work, so keeping this open.
int j = i++ << i++; is still not handled correctly in 9.3, 10.1, and 11.0.
(In reply to Nicolai Josuttis from comment #11) > int j = i++ << i++; > is still not handled correctly in 9.3, 10.1, and 11.0. How come? We warn with -std=c++14 but not -std=c++17, which seems correct.
Oh, sorry, your are right, the example indeed works. BUT: I used in fact a slightly different example (sorry, didn't expect that there is a difference): int main() { int i = 0; int j = i++ << i++; // OK, NO WARNING std::cout << i++ << i++; // still WARNING } see https://wandbox.org/permlink/ioZnOv3oAp5F0BsQ According to my understanding the warning should especially also not come when we pass i++ twice to std::cout (to sequence output was a key goal of this fix in C++17). But may be I am missing something.
(In reply to Nicolai Josuttis from comment #13) > Oh, sorry, your are right, the example indeed works. > BUT: I used in fact a slightly different example > (sorry, didn't expect that there is a difference): > > int main() { > int i = 0; > int j = i++ << i++; // OK, NO WARNING > std::cout << i++ << i++; // still WARNING > } > > see https://wandbox.org/permlink/ioZnOv3oAp5F0BsQ > > According to my understanding the warning should especially > also not come when we pass i++ twice to std::cout > (to sequence output was a key goal of this fix in C++17). > > But may be I am missing something. That would be PR 83028 .