[clang] [Clang][Sema] Print more static_assert exprs (PR #74852)

via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 5 08:54:06 PST 2024


https://github.com/sethp updated https://github.com/llvm/llvm-project/pull/74852

>From f281d34a51f662c934f158e4770774b0dc3588a2 Mon Sep 17 00:00:00 2001
From: Seth Pellegrino <seth at codecopse.net>
Date: Thu, 7 Dec 2023 08:45:51 -0800
Subject: [PATCH 1/3] [Clang][Sema] Print more static_assert exprs

This change introspects more values involved in a static_assert, and
extends the supported set of operators for introspection to include
binary operator method calls.

It's intended to address the use-case where a small static_assert helper
looks something like this (via `constexpr-builtin-bit-cast.cpp`):

```c++
struct int_splicer {
  unsigned x;
  unsigned y;

  constexpr bool operator==(const int_splicer &other) const {
    return other.x == x && other.y == y;
  }
};
```

When used like so:

```c++
constexpr int_splicer got{1, 2};
constexpr int_splicer want{3, 4};
static_assert(got == want);
```

Then we'd expect to get the error:

```
Static assertion failed due to requirement 'got == want'
```

And this change adds the helpful note:

```
Expression evaluates to '{1, 2} == {3, 4}'
```
---
 clang/lib/Sema/SemaDeclCXX.cpp                | 31 ++++++++++++++-----
 .../CXX/class/class.compare/class.eq/p3.cpp   | 20 ++++++------
 .../CXX/class/class.compare/class.rel/p2.cpp  | 10 +++---
 .../over.match.oper/p9-2a.cpp                 |  2 +-
 clang/test/SemaCXX/static-assert-cxx17.cpp    |  2 +-
 5 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index c6218a491aecec..e3d46c3140741b 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17219,6 +17219,13 @@ static bool ConvertAPValueToString(const APValue &V, QualType T,
     OS << "i)";
   } break;
 
+  case APValue::ValueKind::Array:
+  case APValue::ValueKind::Vector:
+  case APValue::ValueKind::Struct: {
+    llvm::raw_svector_ostream OS(Str);
+    V.printPretty(OS, Context, T);
+  } break;
+
   default:
     return false;
   }
@@ -17256,11 +17263,10 @@ static bool UsefulToPrintExpr(const Expr *E) {
 /// Try to print more useful information about a failed static_assert
 /// with expression \E
 void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
-  if (const auto *Op = dyn_cast<BinaryOperator>(E);
-      Op && Op->getOpcode() != BO_LOr) {
-    const Expr *LHS = Op->getLHS()->IgnoreParenImpCasts();
-    const Expr *RHS = Op->getRHS()->IgnoreParenImpCasts();
-
+  const auto Diagnose = [&](const Expr *LHS, const Expr *RHS,
+                            const llvm::StringRef &OpStr) {
+    LHS = LHS->IgnoreParenImpCasts();
+    RHS = RHS->IgnoreParenImpCasts();
     // Ignore comparisons of boolean expressions with a boolean literal.
     if ((isa<CXXBoolLiteralExpr>(LHS) && RHS->getType()->isBooleanType()) ||
         (isa<CXXBoolLiteralExpr>(RHS) && LHS->getType()->isBooleanType()))
@@ -17287,10 +17293,19 @@ void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
                                  DiagSide[I].ValueString, Context);
     }
     if (DiagSide[0].Print && DiagSide[1].Print) {
-      Diag(Op->getExprLoc(), diag::note_expr_evaluates_to)
-          << DiagSide[0].ValueString << Op->getOpcodeStr()
-          << DiagSide[1].ValueString << Op->getSourceRange();
+      Diag(E->getExprLoc(), diag::note_expr_evaluates_to)
+          << DiagSide[0].ValueString << OpStr << DiagSide[1].ValueString
+          << E->getSourceRange();
     }
+  };
+
+  if (const auto *Op = dyn_cast<BinaryOperator>(E);
+      Op && Op->getOpcode() != BO_LOr) {
+    Diagnose(Op->getLHS(), Op->getRHS(), Op->getOpcodeStr());
+  } else if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E);
+             Op && Op->isInfixBinaryOp()) {
+    Diagnose(Op->getArg(0), Op->getArg(1),
+             getOperatorSpelling(Op->getOperator()));
   }
 }
 
diff --git a/clang/test/CXX/class/class.compare/class.eq/p3.cpp b/clang/test/CXX/class/class.compare/class.eq/p3.cpp
index 04db022fe73021..53c4dda133301b 100644
--- a/clang/test/CXX/class/class.compare/class.eq/p3.cpp
+++ b/clang/test/CXX/class/class.compare/class.eq/p3.cpp
@@ -6,11 +6,11 @@ struct A {
 };
 
 static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 3, 4, 5});
-static_assert(A{1, 2, 3, 4, 5} == A{0, 2, 3, 4, 5}); // expected-error {{failed}}
-static_assert(A{1, 2, 3, 4, 5} == A{1, 0, 3, 4, 5}); // expected-error {{failed}}
-static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 0, 4, 5}); // expected-error {{failed}}
-static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 3, 0, 5}); // expected-error {{failed}}
-static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 3, 4, 0}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} == A{0, 2, 3, 4, 5}); // expected-error {{failed}} expected-note {{evaluates to}}
+static_assert(A{1, 2, 3, 4, 5} == A{1, 0, 3, 4, 5}); // expected-error {{failed}} expected-note {{evaluates to}}
+static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 0, 4, 5}); // expected-error {{failed}} expected-note {{evaluates to}}
+static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 3, 0, 5}); // expected-error {{failed}} expected-note {{evaluates to}}
+static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 3, 4, 0}); // expected-error {{failed}} expected-note {{evaluates to}}
 
 struct B {
   int a, b[3], c;
@@ -18,8 +18,8 @@ struct B {
 };
 
 static_assert(B{1, 2, 3, 4, 5} == B{1, 2, 3, 4, 5});
-static_assert(B{1, 2, 3, 4, 5} == B{0, 2, 3, 4, 5}); // expected-error {{failed}}
-static_assert(B{1, 2, 3, 4, 5} == B{1, 0, 3, 4, 5}); // expected-error {{failed}}
-static_assert(B{1, 2, 3, 4, 5} == B{1, 2, 0, 4, 5}); // expected-error {{failed}}
-static_assert(B{1, 2, 3, 4, 5} == B{1, 2, 3, 0, 5}); // expected-error {{failed}}
-static_assert(B{1, 2, 3, 4, 5} == B{1, 2, 3, 4, 0}); // expected-error {{failed}}
+static_assert(B{1, 2, 3, 4, 5} == B{0, 2, 3, 4, 5}); // expected-error {{failed}} expected-note {{evaluates to}}
+static_assert(B{1, 2, 3, 4, 5} == B{1, 0, 3, 4, 5}); // expected-error {{failed}} expected-note {{evaluates to}}
+static_assert(B{1, 2, 3, 4, 5} == B{1, 2, 0, 4, 5}); // expected-error {{failed}} expected-note {{evaluates to}}
+static_assert(B{1, 2, 3, 4, 5} == B{1, 2, 3, 0, 5}); // expected-error {{failed}} expected-note {{evaluates to}}
+static_assert(B{1, 2, 3, 4, 5} == B{1, 2, 3, 4, 0}); // expected-error {{failed}} expected-note {{evaluates to}}
diff --git a/clang/test/CXX/class/class.compare/class.rel/p2.cpp b/clang/test/CXX/class/class.compare/class.rel/p2.cpp
index 90115284d2bd02..07501c6a081841 100644
--- a/clang/test/CXX/class/class.compare/class.rel/p2.cpp
+++ b/clang/test/CXX/class/class.compare/class.rel/p2.cpp
@@ -10,15 +10,15 @@ namespace Rel {
     friend bool operator>=(const A&, const A&) = default;
   };
   static_assert(A{0} < A{1});
-  static_assert(A{1} < A{1}); // expected-error {{failed}}
+  static_assert(A{1} < A{1}); // expected-error {{failed}} expected-note {{'{1} < {1}'}}
   static_assert(A{0} <= A{1});
   static_assert(A{1} <= A{1});
-  static_assert(A{2} <= A{1}); // expected-error {{failed}}
+  static_assert(A{2} <= A{1}); // expected-error {{failed}} expected-note {{'{2} <= {1}'}}
   static_assert(A{1} > A{0});
-  static_assert(A{1} > A{1}); // expected-error {{failed}}
+  static_assert(A{1} > A{1}); // expected-error {{failed}} expected-note {{'{1} > {1}'}}
   static_assert(A{1} >= A{0});
   static_assert(A{1} >= A{1});
-  static_assert(A{1} >= A{2}); // expected-error {{failed}}
+  static_assert(A{1} >= A{2}); // expected-error {{failed}} expected-note {{'{1} >= {2}'}}
 
   struct B {
     bool operator<=>(B) const = delete; // expected-note 4{{deleted here}} expected-note-re 8{{candidate {{.*}} deleted}}
@@ -49,7 +49,7 @@ namespace NotEqual {
     friend bool operator!=(const A&, const A&) = default;
   };
   static_assert(A{1} != A{2});
-  static_assert(A{1} != A{1}); // expected-error {{failed}}
+  static_assert(A{1} != A{1}); // expected-error {{failed}} expected-note {{'{1} != {1}'}}
 
   struct B {
     bool operator==(B) const = delete; // expected-note {{deleted here}} expected-note-re 2{{candidate {{.*}} deleted}}
diff --git a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p9-2a.cpp b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p9-2a.cpp
index 95d6a55aee66a1..8f31e8947a768c 100644
--- a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p9-2a.cpp
+++ b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p9-2a.cpp
@@ -33,7 +33,7 @@ struct Y {};
 constexpr bool operator==(X x, Y) { return x.equal; }
 
 static_assert(X{true} == Y{});
-static_assert(X{false} == Y{}); // expected-error {{failed}}
+static_assert(X{false} == Y{}); // expected-error {{failed}} expected-note{{'{false} == {}'}}
 
 // x == y -> y == x
 static_assert(Y{} == X{true});
diff --git a/clang/test/SemaCXX/static-assert-cxx17.cpp b/clang/test/SemaCXX/static-assert-cxx17.cpp
index 41a7b025d0eb75..1d78915aa13e18 100644
--- a/clang/test/SemaCXX/static-assert-cxx17.cpp
+++ b/clang/test/SemaCXX/static-assert-cxx17.cpp
@@ -94,7 +94,7 @@ void foo6() {
   // expected-error at -1{{static assertion failed due to requirement '(const X<int> *)nullptr'}}
   static_assert(static_cast<const X<typename T::T> *>(nullptr));
   // expected-error at -1{{static assertion failed due to requirement 'static_cast<const X<int> *>(nullptr)'}}
-  static_assert((const X<typename T::T>[]){} == nullptr);
+  static_assert((const X<typename T::T>[]){} == nullptr); // expected-note{{expression evaluates to '{} == nullptr'}}
   // expected-error at -1{{static assertion failed due to requirement '(const X<int>[0]){} == nullptr'}}
   static_assert(sizeof(X<decltype(X<typename T::T>().X<typename T::T>::~X())>) == 0);
   // expected-error at -1{{static assertion failed due to requirement 'sizeof(X<void>) == 0'}} \

>From 2fd5fc464868a682cb4b66ea4e97a6a7100ab120 Mon Sep 17 00:00:00 2001
From: Seth Pellegrino <seth at codecopse.net>
Date: Fri, 5 Jan 2024 08:49:02 -0800
Subject: [PATCH 2/3] add test for review discussion

---
 .../SemaCXX/static-assert-diagnostics.cpp     | 117 ++++++++++++++++++
 1 file changed, 117 insertions(+)
 create mode 100644 clang/test/SemaCXX/static-assert-diagnostics.cpp

diff --git a/clang/test/SemaCXX/static-assert-diagnostics.cpp b/clang/test/SemaCXX/static-assert-diagnostics.cpp
new file mode 100644
index 00000000000000..432a3f4bd54656
--- /dev/null
+++ b/clang/test/SemaCXX/static-assert-diagnostics.cpp
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 -std=c++2a -verify %s
+
+struct A {
+  int a, b[3], c;
+  bool operator==(const A&) const = default;
+};
+
+constexpr auto a0 = A{0, 0, 3, 4, 5};
+
+// expected-note at +1 {{evaluates to '{0, {0, 3, 4}, 5} == {1, {2, 3, 4}, 5}'}}
+static_assert(a0 == A{1, {2, 3, 4}, 5}); // expected-error {{failed}}
+
+struct _arr {
+  const int b[3];
+  constexpr bool operator==(const int rhs[3]) const {
+    for (unsigned i = 0; i < sizeof(b) / sizeof(int); i++)
+      if (b[i] != rhs[i])
+        return false;
+    return true;
+  }
+};
+
+// TODO[seth] syntactically sort of valid but almost entirely unusuable
+// (it's an int *, not an int [3] )
+// constexpr int _[3] = {...}; would work, but that's not piecewise substitutable
+// maybe it's ok? I mean, not like we can do better really...
+constexpr auto _ = (int[3]){2, 3, 4};
+
+// output: '{{2, 3, 4}} == {0, 3, 4}'  (the `{{` breaks VerifyDiagnosticConsumer::ParseDirective)
+// expected-note at +1 {{evaluates to}}
+static_assert(_arr{2, 3, 4} == a0.b); // expected-error {{failed}}
+
+struct B {
+  int a, c; // named the same just to keep things fresh
+  bool operator==(const B&) const = default;
+};
+
+// expected-note at +1 {{evaluates to '{7, 6} == {8, 6}'}}
+static_assert(B{7, 6} == B{8, 6}); // expected-error {{failed}}
+
+typedef int v4si __attribute__((__vector_size__(16)));
+
+struct C: A, B {
+  enum { E1, E2 } e;
+  bool operator==(const C&) const = default;
+};
+
+constexpr auto cc = C{A{1, {2, 3, 4}, 5}, B{7, 6}, C::E1};
+
+// actually '{{1, {2, 3, 4}, 5}, {7, 6}, 0} == {{0, {0, 3, 4}, 5}, {5, 0}, 1}'  (the `{{` breaks VerifyDiagnosticConsumer::ParseDirective)
+// expected-note at +1 {{evaluates to}}
+static_assert(cc == C{a0, {5}, C::E2}); // expected-error {{failed}}
+
+// this little guy? oh, I wouldn't worry about this little guy
+namespace std {
+template <class To, class From>
+constexpr To bit_cast(const From &from) {
+  static_assert(sizeof(To) == sizeof(From));
+  return __builtin_bit_cast(To, from);
+}
+} // namespace std
+
+typedef int v4si __attribute__((__vector_size__(16)));
+
+struct V {
+  v4si v;
+
+  // doesn't work
+  // vectors are not contextually convertable to `bool`, and
+  // `==` on vectors produces a vector of element-wise results
+  // bool operator==(const V&) const = default;
+
+  constexpr bool operator==(const V& rhs) const {
+    // doesn't work
+    // __builtin_reduce_and is not valid in a constant expression
+    // return __builtin_reduce_and(b == rhs.b) && __builtin_reduce_and(v == rhs.v);
+
+    // also doesn't work
+    // surprisingly, b[0] is also not valid in a constant expression (nor v[0])
+    // return b[0] == rhs.b[0] && ...
+
+    struct cmp {
+      unsigned char v [sizeof(v4si)];
+      bool operator==(const cmp&) const = default;
+    };
+    return std::bit_cast<cmp>(v) == std::bit_cast<cmp>(rhs.v);
+  };
+
+};
+// todo[seth] do I cause infinite evaluator recursion if I move this up into the function above?
+static_assert(V{1, 2, 3, 4} == V{1, 2, 3, 4});
+
+// '{{1, 2, 3, 4}} == {{1, 2, 0, 4}}'
+// expected-note at +1 {{evaluates to}}
+static_assert(V{1, 2, 3, 4} == V{1, 2, 0, 4}); // expected-error {{failed}}
+
+constexpr auto v = (v4si){1, 2, 3, 4};
+constexpr auto vv = V{{1, 2, 3, 4}};
+
+
+// there appears to be no constexpr-compatible way to write an == for
+// two `bool4`s at this time, since std::bit_cast doesn't support it
+// typedef bool bool4 __attribute__((ext_vector_type(4)));
+
+// so we use a bool8
+typedef bool bool8 __attribute__((ext_vector_type(8)));
+
+struct BV {
+  bool8 b;
+  constexpr bool operator==(const BV& rhs) const {
+    return std::bit_cast<unsigned char>(b) == std::bit_cast<unsigned char>(rhs.b);
+  }
+};
+
+// '{{false, true, false, false, false, false, false, false}} == {{true, false, false, false, false, false, false, false}}'
+// expected-note at +1 {{evaluates to}}
+static_assert(BV{{0, 1}} == BV{{1, 0}}); // expected-error {{failed}}

>From db9118fc6f14e84a518e3ee0990041bac8979522 Mon Sep 17 00:00:00 2001
From: Seth Pellegrino <seth at codecopse.net>
Date: Fri, 5 Jan 2024 08:53:23 -0800
Subject: [PATCH 3/3] fixup!: remove notes to self unrelated to PR

Oops, those were just things I was curious about, not meant to be left
in.
---
 clang/test/SemaCXX/static-assert-diagnostics.cpp | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/clang/test/SemaCXX/static-assert-diagnostics.cpp b/clang/test/SemaCXX/static-assert-diagnostics.cpp
index 432a3f4bd54656..a92a6d5fb08c3c 100644
--- a/clang/test/SemaCXX/static-assert-diagnostics.cpp
+++ b/clang/test/SemaCXX/static-assert-diagnostics.cpp
@@ -20,12 +20,6 @@ struct _arr {
   }
 };
 
-// TODO[seth] syntactically sort of valid but almost entirely unusuable
-// (it's an int *, not an int [3] )
-// constexpr int _[3] = {...}; would work, but that's not piecewise substitutable
-// maybe it's ok? I mean, not like we can do better really...
-constexpr auto _ = (int[3]){2, 3, 4};
-
 // output: '{{2, 3, 4}} == {0, 3, 4}'  (the `{{` breaks VerifyDiagnosticConsumer::ParseDirective)
 // expected-note at +1 {{evaluates to}}
 static_assert(_arr{2, 3, 4} == a0.b); // expected-error {{failed}}
@@ -87,7 +81,6 @@ struct V {
   };
 
 };
-// todo[seth] do I cause infinite evaluator recursion if I move this up into the function above?
 static_assert(V{1, 2, 3, 4} == V{1, 2, 3, 4});
 
 // '{{1, 2, 3, 4}} == {{1, 2, 0, 4}}'



More information about the cfe-commits mailing list