[Sema] SequenceChecker: Fix handling of operator ||, && and ?:

The current handling of the operators ||, && and ?: has a number of false
positive and false negative. The issues for operator || and && are:

1. We need to add sequencing regions for the LHS and RHS as is done for the
   comma operator. Not doing so causes false positives in expressions like
   `((a++, false) || (a++, false))` (from PR39779, see PR22197 for another
    example).

2. In the current implementation when the evaluation of the LHS fails, the RHS
   is added to a worklist to be processed later. This results in false negatives
   in expressions like `(a && a++) + a`.

Fix these issues by introducing sequencing regions for the LHS and RHS, and by
not deferring the visitation of the RHS.

The issues with the ternary operator ?: are similar, with the added twist that
we should not warn on expressions like `(x ? y += 1 : y += 2)` since exactly
one of the 2nd and 3rd expression is going to be evaluated, but we should still
warn on expressions like `(x ? y += 1 : y += 2) = y`.

Differential Revision: https://reviews.llvm.org/D57747

Reviewed By: rsmith

1 files changed, 8 insertions, 7 deletions

diff --git a/clang/test/Sema/warn-unsequenced.c b/clang/test/Sema/warn-unsequenced.c
index 9654cda9240..fd0227b479a 100644
--- a/clang/test/Sema/warn-unsequenced.c
+++ b/clang/test/Sema/warn-unsequenced.c
@@ -40,18 +40,18 @@ void test() {
   A agg1 = { a++, a++ }; // expected-warning {{multiple unsequenced modifications}}
   A agg2 = { a++ + a, a++ }; // expected-warning {{unsequenced modification and access}}
 
-  (xs[2] && (a = 0)) + a; // ok
+  (xs[2] && (a = 0)) + a; // expected-warning {{unsequenced modification and access to 'a'}}
   (0 && (a = 0)) + a; // ok
   (1 && (a = 0)) + a; // expected-warning {{unsequenced modification and access}}
 
-  (xs[3] || (a = 0)) + a; // ok
+  (xs[3] || (a = 0)) + a; // expected-warning {{unsequenced modification and access to 'a'}}
   (0 || (a = 0)) + a; // expected-warning {{unsequenced modification and access}}
   (1 || (a = 0)) + a; // ok
 
-  (xs[4] ? a : ++a) + a; // ok
+  (xs[4] ? a : ++a) + a; // expected-warning {{unsequenced modification and access to 'a'}}
   (0 ? a : ++a) + a; // expected-warning {{unsequenced modification and access}}
   (1 ? a : ++a) + a; // ok
-  (xs[5] ? ++a : ++a) + a; // FIXME: warn here
+  (xs[5] ? ++a : ++a) + a; // expected-warning {{unsequenced modification and access to 'a'}}
 
   (++a, xs[6] ? ++a : 0) + a; // expected-warning {{unsequenced modification and access}}
 
@@ -73,10 +73,11 @@ void test() {
   // unconditional.
   a = a++ && f(a, a);
 
-  // This has undefined behavior if a != 0. FIXME: We should diagnose this.
-  (a && a++) + a;
+  // This has undefined behavior if a != 0.
+  (a && a++) + a; // expected-warning {{unsequenced modification and access to 'a'}}
 
-  (xs[7] && ++a) * (!xs[7] && ++a); // ok
+  // FIXME: Find a way to avoid warning here.
+  (xs[7] && ++a) * (!xs[7] && ++a); // expected-warning {{multiple unsequenced modifications to 'a'}}
 
   xs[0] = (a = 1, a); // ok