[PATCH] D131070: [clang][sema] Fix collectConjunctionTerms()

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added a reviewer: aaron.ballman.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Consider:

A == 5 && A != 5

IfA is 5, the old collectConjunctionTerms() would call itself again for
the LHS (which it ignores), then  the RHS (which it also ignores) and
then just return without ever adding anything to the Terms array.

For example, there's a test case in `clang/test/SemaCXX/recovery-expr-type.cpp`:

  namespace test13 {
  enum Circular {             // expected-note {{not complete until the closing '}'}}
    Circular_A = Circular(1), // expected-error {{'Circular' is an incomplete type}}
  };
  // Enumerators can be evaluated (they evaluate as zero, but we don't care).
  static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}}
  }

which currently prints:

  test2.cpp:6:1: error: static assertion failed due to requirement 'Circular_A == 0 && Circular_A != 0':
  static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}}
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  2 errors generated.

(other diagnostics omitted)

but after this change prints:

  test2.cpp:6:1: error: static assertion failed due to requirement 'Circular_A != 0':
  static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}}
  ^                                ~~~~~~~~~~~~~~~
  2 errors generated.

The patch depends on https://reviews.llvm.org/D130894 because of the note it adds, but that's not necessary. It's just easier because they are both in my local tree.

I wanted to add Douglas Gregor as reviewer as well but seems like he isn't around anymore.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131070

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===================================================================
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -149,7 +149,8 @@
   Circular_A = Circular(1), // expected-error {{'Circular' is an incomplete type}}
 };
 // Enumerators can be evaluated (they evaluate as zero, but we don't care).
-static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}}
+static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}} \
+                                                       // expected-note {{evaluates to 0}}
 }
 
 namespace test14 {
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -3585,9 +3585,8 @@
     if (BinOp->getOpcode() == BO_LAnd) {
       collectConjunctionTerms(BinOp->getLHS(), Terms);
       collectConjunctionTerms(BinOp->getRHS(), Terms);
+      return;
     }
-
-    return;
   }
 
   Terms.push_back(Clause);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D131070.449650.patch
Type: text/x-patch
Size: 1144 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220803/99ea2389/attachment-0001.bin>