[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415
Andrew Pinski changed:
What|Removed |Added
See Also||https://gcc.gnu.org/bugzill
||a/show_bug.cgi?id=83028
--- Comment #14 from Andrew Pinski ---
(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 .
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415
--- Comment #13 from Nicolai Josuttis ---
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.
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 Marek Polacek changed: What|Removed |Added CC||mpolacek at gcc dot gnu.org --- Comment #12 from Marek Polacek --- (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.
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 Nicolai Josuttis changed: What|Removed |Added CC||nico at josuttis dot de --- Comment #11 from Nicolai Josuttis --- int j = i++ << i++; is still not handled correctly in 9.3, 10.1, and 11.0.
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 --- Comment #10 from Jakub Jelinek --- Partially fixed, needs more work, so keeping this open.
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 --- Comment #9 from Jakub Jelinek --- Author: jakub Date: Tue Aug 27 12:37:30 2019 New Revision: 274952 URL: https://gcc.gnu.org/viewcvs?rev=274952=gcc=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
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 --- Comment #8 from Jakub Jelinek --- Oh, and need to look also at CALL_EXPR.
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415
--- Comment #7 from Jakub Jelinek ---
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), _before, _nosp, NULL_TREE);
verify_tree (TREE_OPERAND (x, 0), _list3, _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 (_before, tmp_list3, x, 1);
- warn_for_collisions (tmp_before);
+ if (cxx_dialect < cxx17)
+ {
+ add_tlist (_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.jj2011-07-11
10:39:37.0 +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.
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 --- Comment #6 from Jakub Jelinek --- Created attachment 46702 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46702=edit gcc10-pr91415-1.patch Let's handle this incrementally, from the easiest cases to the hardest.
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 --- Comment #5 from Marek Polacek --- 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
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 Marek Polacek changed: What|Removed |Added Keywords||diagnostic Status|UNCONFIRMED |NEW Last reconfirmed||2019-08-10 Ever confirmed|0 |1
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415
--- Comment #4 from Jakub Jelinek ---
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++];
}
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 --- Comment #3 from Jakub Jelinek --- --- 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), _before, _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.
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org,
||jason at gcc dot gnu.org,
||mpolacek at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek ---
So shall we emit no warnings with -std=c++17 -Wsequence-point on:
struct S { int a[10]; };
void
foo (int i, int ()[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?
[Bug c++/91415] Invalid warning for C++17 sequencing of shift operator E1<
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91415 --- Comment #1 from Maxim Egorushkin --- gcc-9.1 produces the same warning.
