[clang] Add some CodeGen tests for CWG 2xx issues #80823

Endilll · 2024-02-06T10:53:27Z

This patch covers CWG issues
201,
210,
292.

CWG208 is not covered, as it actually requires a libcxxabi test.

Resolution of CWG292 has been superseded by P0145R3 "Refining Expression Evaluation Order for
Idiomatic C++" (see changes to paragraph 5.3.4/18).

This patch covers CWG issues [201](https://cplusplus.github.io/CWG/issues/201.html), [210](https://cplusplus.github.io/CWG/issues/210.html), [292](https://cplusplus.github.io/CWG/issues/292.html). [CWG208](https://cplusplus.github.io/CWG/issues/208.html) is not covered, as it actually requires a libcxxabi test.

llvmbot · 2024-02-06T10:53:45Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Vlad Serebrennikov (Endilll)

Changes

This patch covers CWG issues
201,
210,
292.

CWG208 is not covered, as it actually requires a libcxxabi test.

Full diff: https://github.com/llvm/llvm-project/pull/80823.diff

5 Files Affected:

(added) clang/test/CXX/drs/dr201.cpp (+43)
(added) clang/test/CXX/drs/dr210.cpp (+41)
(added) clang/test/CXX/drs/dr292.cpp (+38)
(modified) clang/test/CXX/drs/dr2xx.cpp (+3-3)
(modified) clang/www/cxx_dr_status.html (+4-4)

diff --git a/clang/test/CXX/drs/dr201.cpp b/clang/test/CXX/drs/dr201.cpp
new file mode 100644
index 00000000000000..48495d337e00ff
--- /dev/null
+++ b/clang/test/CXX/drs/dr201.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++98 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++11 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+
+// RUN: %clang_cc1 -std=c++98 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++11 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+
+#if __cplusplus == 199711L
+#define NOTHROW throw()
+#else
+#define NOTHROW noexcept(true)
+#endif
+
+namespace dr201 { // dr201: 2.8
+
+struct A {
+  ~A() NOTHROW {}
+};
+
+struct B {
+  B(A) NOTHROW {}
+  ~B() NOTHROW {}
+};
+
+void foo() {
+  B b = A();
+}
+
+// CHECK-LABEL: define {{.*}} void @dr201::foo()
+// CHECK:         call void @dr201::A::~A()
+// CHECK:         call void @dr201::B::~B()
+// CHECK-LABEL: }
+
+} // namespace dr201
diff --git a/clang/test/CXX/drs/dr210.cpp b/clang/test/CXX/drs/dr210.cpp
new file mode 100644
index 00000000000000..c19771229596f1
--- /dev/null
+++ b/clang/test/CXX/drs/dr210.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++98 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++11 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+
+// RUN: %clang_cc1 -std=c++98 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++11 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+
+namespace dr210 { // dr210: 2.7
+struct B {
+  B();
+  virtual ~B();
+};
+
+struct D : B {
+  int i;
+  D();
+};
+
+void toss(const B* b) {
+  throw *b;
+}
+
+void f() {
+  const D d;
+  toss(&d);
+}
+
+// CHECK-LABEL: define {{.*}} void @dr210::toss(dr210::B const*)
+// CHECK:         call ptr @__cxa_allocate_exception(i64 8)
+// CHECK-LABEL: }
+
+} // namespace dr210
diff --git a/clang/test/CXX/drs/dr292.cpp b/clang/test/CXX/drs/dr292.cpp
new file mode 100644
index 00000000000000..e59ce2652aba97
--- /dev/null
+++ b/clang/test/CXX/drs/dr292.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -std=c++98 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++11 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+
+// RUN: %clang_cc1 -std=c++98 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++11 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+
+namespace dr292 { // dr292: 2.9
+
+extern int g();
+
+struct A {
+  A(int) throw() {}
+};
+
+void f() {
+  new A(g());
+}
+
+// CHECK-LABEL: define {{.*}} void @dr292::f()()
+// CHECK:         %[[CALL:.+]] = call {{.*}} @operator new(unsigned long)({{.*}})
+// CHECK:         invoke {{.*}} i32 @dr292::g()()
+// CHECK-NEXT:           to {{.*}} unwind label %lpad
+// CHECK-LABEL: lpad:
+// CHECK:         call void @operator delete(void*)(ptr {{.*}} %[[CALL]])
+// CHECK-LABEL: eh.resume:
+// CHECK-LABEL: }
+
+} // namespace dr292
diff --git a/clang/test/CXX/drs/dr2xx.cpp b/clang/test/CXX/drs/dr2xx.cpp
index 1a3ac532f93b26..cbb8734e10c649 100644
--- a/clang/test/CXX/drs/dr2xx.cpp
+++ b/clang/test/CXX/drs/dr2xx.cpp
@@ -26,7 +26,7 @@ namespace dr200 { // dr200: dup 214
   }
 }
 
-// dr201 FIXME: write codegen test
+// dr201 is in dr201.cpp
 
 namespace dr202 { // dr202: 3.1
   template<typename T> T f();
@@ -76,7 +76,7 @@ namespace dr209 { // dr209: 3.2
   };
 }
 
-// dr210 FIXME: write codegen test
+// dr210 is in dr210.cpp
 
 namespace dr211 { // dr211: yes
   struct A {
@@ -1188,7 +1188,7 @@ namespace dr289 { // dr289: yes
 
 // dr290: na
 // dr291: dup 391
-// dr292 FIXME: write a codegen test
+// dr292 is in dr292.cpp
 
 namespace dr294 { // dr294: no
   void f() throw(int);
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 3e13a4d89ef989..109566a12da030 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -782,7 +782,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/124.html">124</a></td>
     <td>CD1</td>
     <td>Lifetime of temporaries in default initialization of class arrays</td>
-    <td class="unknown" align="center">Duplicate of <a href="#201">201</a></td>
+    <td class="full" align="center">Duplicate of <a href="#201">201</a></td>
   </tr>
   <tr id="125">
     <td><a href="https://cplusplus.github.io/CWG/issues/125.html">125</a></td>
@@ -1244,7 +1244,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/201.html">201</a></td>
     <td>CD1</td>
     <td>Order of destruction of temporaries in initializers</td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="full" align="center">Clang 2.8</td>
   </tr>
   <tr id="202">
     <td><a href="https://cplusplus.github.io/CWG/issues/202.html">202</a></td>
@@ -1299,7 +1299,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/210.html">210</a></td>
     <td>TC1</td>
     <td>What is the type matched by an exception handler?</td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="full" align="center">Clang 2.7</td>
   </tr>
   <tr id="211">
     <td><a href="https://cplusplus.github.io/CWG/issues/211.html">211</a></td>
@@ -1792,7 +1792,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/292.html">292</a></td>
     <td>CD3</td>
     <td>Deallocation on exception in <TT>new</TT> before arguments evaluated</td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="full" align="center">Clang 2.9</td>
   </tr>
   <tr class="open" id="293">
     <td><a href="https://cplusplus.github.io/CWG/issues/293.html">293</a></td>

cor3ntin · 2024-02-06T11:11:11Z

clang/test/CXX/drs/dr201.cpp

+// RUN: %clang_cc1 -std=c++98 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++11 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O0 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+
+// RUN: %clang_cc1 -std=c++98 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++11 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK


This is way too many tests imo!

Unfortunately, CodeGen tests cover both language frontend and codegen, so we have 7 major frontend codepaths times 2 major codegen codepaths to test.

We do not write -O3 tests because that's a concern for backend folks.
And generally, unless there have been significant changes there is no value in testing all standard versions

We do not write -O3 tests because that's a concern for backend folks.

Clang's CodeGen is also concerned whether optimization are enabled or not in 55 places. I don't want to miss those codepaths.

And generally, unless there have been significant changes there is no value in testing all standard versions

Those are not general tests, but defect report tests. I believe they should be held to higher standards.

I think a good litmus test as to whether a test belong in Clang or not is, "if the test fail, can we fix it?"
And here we would not be able to, lowering IR is not in the purview of the front end, so this is not testing the front end.

And sure, there is a wider question here: Does the LLVM middle-end implements and test the semantics of C++
and those core issues that are relevant?

But regardless of the answer that question, putting such test here would not serve to increase our coverage.
We test that clang produces good IR, and we trust that given the IR of a well-formed program LLVM will preserve the semantics.

Ditto we should not test libc++, libc++abi, etc

And sure, there is a wider question here: Does the LLVM middle-end implements and test the semantics of C++ and those core issues that are relevant?

CC @lattner as general LLVM code owner, both for an answer to this question, but also as a request to find a dedicated code owner within LLVM that the Clang community can interface with regarding C and C++ conformance from the LLVM side of things. (For example, we really need someone on the LLVM side who we can talk to regarding changes to the memory object model in both C and C++ as the languages evolve so that we can plan a path forward accordingly.)

I think a good litmus test as to whether a test belong in Clang or not is, "if the test fail, can we fix it?"
And here we would not be able to, lowering IR is not in the purview of the front end, so this is not testing the front end.

Lowering AST to IR is in purview of Clang, and we're responsible that this happens correctly (even if no single individual in this conversation is an expert in that part of Clang). I hope everyone here agrees that middle-end folks are not responsible for the correctness of our CodeGen from the C++ language semantics perspective.

And sure, there is a wider question here: Does the LLVM middle-end implements and test the semantics of C++
and those core issues that are relevant?

No, there is no such question, and there wasn't at any point in DR testing discussion. CodeGen DR tests are interested in Clang's CodeGen, and I do my best to ensure that middle-end does not interfere. If you think middle-end is still involved somehow, let me know how this can be fixed.

Keep in mind that changes get tested in multiple places (when done correctly), so when someone implements a DR (or a feature with the DR already applied), they should have test coverage in clang/test/CodeGen* as well as in clang/test/Sema*, and here in the DR test file. We don't need to re-test things covered by other tests, and sometimes the coverage makes more sense elsewhere than a DR test.

This holds for the "usual" DR tests as well. They sure duplicate some of Sema tests. Same goes for the whole CXX directory. On top of the fact that calculating coverage is both time- and resource- consuming, making automatic duplicate discovery a serious burden, regular Sema and CodeGen tests are not necessarily designed to cover use cases DR tests cover. Which means they can change in a ways not fit for DR testing. So we should be careful about depending on them.

I tend to agree with @cor3ntin that this may be going overboard in terms of test runs.

It wouldn't take too much time for a list of language modes to have the same length. Is cutting a number of language modes DR tests are run in also on the table because of arbitrary limits (as I see them at the moment)?

CC @rjmccall and @efriedma-quic as Clang codegen code owners.

Lowering AST to IR is in purview of Clang, and we're responsible that this happens correctly (even if no single individual in this conversation is an expert in that part of Clang). I hope everyone here agrees that middle-end folks are not responsible for the correctness of our CodeGen from the C++ language semantics perspective.

There's a handshake involved between Clang and LLVM, so I don't think there's one responsible party for correctness here. Part of the correctness comes from Clang lowering to sensible LLVM IR, but we sometimes need LLVM folks to tell us what that sensible IR is or to provide different tools for us if existing IR doesn't suffice. However, in terms of DR testing, I think most of the effort should be expended on Clang's side of things.

It wouldn't take too much time for a list of language modes to have the same length. Is cutting a number of language modes DR tests are run in also on the table because of arbitrary limits (as I see them at the moment)?

IMO, we want to find a happy middle ground between testing too much and testing too little. The issues with testing too little are obvious, but the issue from over-testing is that we waste significant resources for little benefit. Everyone runs the test suite (precommit CI, postcommit CI, and developers on their local machines) so if we increase the test time significantly without discernible benefit, that can lead to death-by-a-thousand-cuts problems.

FWIW, I wasn't insisting on code coverage measurements. I was more pointing out that I think we should not add the extra RUN lines for -O3 while speculating that it increases test coverage. We should have some reason to believe it increases test coverage, and we might want to consider splitting those tests out into their own file so we don't run 100 DR tests in two optimization modes for every standard version we're testing.

We should have some reason to believe it increases test coverage, and we might want to consider splitting those tests out into their own file so we don't run 100 DR tests in two optimization modes for every standard version we're testing.

But... every codegen tests is in its own file already because of an entirely different concern. I don't see us being able to combine them into a single file for foreseeable future. So running 100 DRs with those additional flags is not on the table.

"DR tests" are not subject to a higher standard than other tests. Many tests are functionality tests for basic specified behavior and are not somehow of lesser significance. This sort of exhaustive testing of configurations would be reasonable for an external conformance test suite, in which case the general structure of the test suite would run every test in many different configurations and the individual tests would just note any expected differences by configuration. However, the Clang regression test suite is not a conformance test suite; it is a set of targeted functionality tests which is designed to detect bugs, regressions, and other behavior changes with tolerably low overhead, given that it is repeatedly run at high frequency in CI and developer testing. Tests should therefore be written with reasonable human knowledge rather than using brute force. You can and should test as many configurations in a test as your knowledge of the code suggests that it is likely that the compiler will take different code paths and/or generate different code patterns for, but otherwise you should not test things redundantly.

cor3ntin · 2024-02-06T11:13:35Z

clang/test/CXX/drs/dr201.cpp

+#define NOTHROW noexcept(true)
+#endif
+
+namespace dr201 { // dr201: 2.8


What is tested here?
My understanding is that this issue deal with the sequence of destructions in

auto foo[2] = {A(temp()), B(temp())};

I'll update the test to make it more clear, but I test whether a temporary is destroyed at the end of full expression. Which I believe is what 201 is about, contrary to its title.

I'll update the test to make it more clear, but I test whether a temporary is destroyed at the end of full expression. Which I believe is what 201 is about, contrary to its title.

TBH, I think this is one of those "textual pendentry" type DRs where two sentences were inconsistent. It is complaining that in the expression: B b = A(); whether the call to B::B is 'part' of the full expression (so the deletion of the temporary isn't really meaningful).

It was never contentous based on my reading whether the temporary was deleted 'end of statement' (in fact, it was, in 1 sentence deleted AFTER init, which was AFTER end of statement, but that wasn't the case in the other sentence).

Because of this, I think your test is reasonably fine, except I'd add a check for the call to B::B before A::~A.

Also, it might be valuable to get the 'default argument to a default constructor called in array' is deleted BEFORE the initialization of the array.

I added check for B::B().
I also added a test for related CWG124 to #80338

clang/test/CXX/drs/dr210.cpp

cor3ntin · 2024-02-06T11:23:24Z

clang/test/CXX/drs/dr292.cpp

+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+// RUN: %clang_cc1 -std=c++2c %s -triple x86_64-linux-gnu -emit-llvm -disable-llvm-passes -O3 -o - -fexceptions -fcxx-exceptions -pedantic-errors | llvm-cxxfilt -n | FileCheck %s --check-prefixes CHECK
+
+namespace dr292 { // dr292: 2.9


I think an additional test
new A(no_except()) checking the absence of delete might also be good

I updated PR description to mention that resolution to 292 has been superseded by P0145R3. The latter is the wording I'm testing: we're making sure that we pass to deallocation function what was returned by allocation function, and making sure that potentially-throwing expression is evaluated in-between.

In new A(no_except()) initializer can't throw, and there's no need figure out what to pass to deallocation function, so I consider it out of scope of the CWG issue.

- removed testing at O3 - updated 201 test

erichkeane · 2024-02-09T14:47:38Z

clang/test/CXX/drs/dr201.cpp

+#define NOTHROW noexcept(true)
+#endif
+
+namespace dr201 { // dr201: 2.8


I'll update the test to make it more clear, but I test whether a temporary is destroyed at the end of full expression. Which I believe is what 201 is about, contrary to its title.

TBH, I think this is one of those "textual pendentry" type DRs where two sentences were inconsistent. It is complaining that in the expression: B b = A(); whether the call to B::B is 'part' of the full expression (so the deletion of the temporary isn't really meaningful).

It was never contentous based on my reading whether the temporary was deleted 'end of statement' (in fact, it was, in 1 sentence deleted AFTER init, which was AFTER end of statement, but that wasn't the case in the other sentence).

Because of this, I think your test is reasonably fine, except I'd add a check for the call to B::B before A::~A.

Also, it might be valuable to get the 'default argument to a default constructor called in array' is deleted BEFORE the initialization of the array.

erichkeane · 2024-02-09T14:56:47Z

clang/test/CXX/drs/dr210.cpp

+}
+
+// CHECK-LABEL: define {{.*}} void @dr210::toss(dr210::B const*)
+// CHECK:         call ptr @__cxa_allocate_exception(i64 8)


Sorry, I don't get how we're testing this here. Why is checking it for 'size 8' going to work here?

I set up B and D in a way that the former has size of 8 bytes, and the latter has 16 due to additional int.
You think it's still not a good way to ensure that we throw object based on their static type?

I might suggest making it so that "B" is not 8 bytes as I was afraid it was actually just doing a pointer there (which it WAS... just not to what I was afraid it was doing)... A comment would be helpful as well.

However, this line would likely be worth testing as well: call void @__cxa_throw(ptr %exception, ptr @typeinfo for B, ptr @B::~B()) #7

That shows the actual throw, and has the typeinfo in it.

Also add a couple of static asserts, and remove unnecessary function

erichkeane

LGTM, make sure @cor3ntin is happy too though.

erichkeane · 2024-02-09T16:52:01Z

clang/test/CXX/drs/dr210.cpp

+#pragma clang diagnostic ignored "-Wvariadic-macros"
+#define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
+#pragma clang diagnostic pop
+#endif


Wowzers, lot of work to make this happen, but thank you for doing it!

cor3ntin

LGTM

Endilll added 2 commits February 6, 2024 13:23

Merge remote-tracking branch 'upstream/main' into cwg2xx-codegen

f7a0d28

Endilll added test-suite c++ clang:codegen IR generation bugs: mangling, exceptions, etc. labels Feb 6, 2024

llvmbot added the clang Clang issues not falling into any other category label Feb 6, 2024

cor3ntin reviewed Feb 6, 2024

View reviewed changes

Endilll requested review from AaronBallman and erichkeane February 6, 2024 11:23

Address feedback

d9ee822

- removed testing at O3 - updated 201 test

Endilll changed the title ~~[Clang] Add some CodeGen tests for CWG 2xx issues~~ [clang] Add some CodeGen tests for CWG 2xx issues Feb 9, 2024

erichkeane reviewed Feb 9, 2024

View reviewed changes

Endilll added 2 commits February 9, 2024 19:21

Address Erich's feedback

4c8e542

Incorporate Erich's suggestion into CWG210 test

964b9df

Also add a couple of static asserts, and remove unnecessary function

erichkeane approved these changes Feb 9, 2024

View reviewed changes

Endilll added 2 commits February 12, 2024 21:16

Merge remote-tracking branch 'upstream/main' into cwg2xx-codegen

7147c04

Merge remote-tracking branch 'upstream/main' into cwg2xx-codegen

323d41e

cor3ntin approved these changes Feb 13, 2024

View reviewed changes

Endilll merged commit 346e7c7 into llvm:main Feb 13, 2024

Endilll deleted the cwg2xx-codegen branch February 13, 2024 08:17

[clang] Add some CodeGen tests for CWG 2xx issues #80823

[clang] Add some CodeGen tests for CWG 2xx issues #80823

Uh oh!

Conversation

Endilll commented Feb 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cor3ntin Feb 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjmccall Feb 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Endilll commented Feb 6, 2024 •

edited

Loading

llvmbot commented Feb 6, 2024 •

edited

Loading

cor3ntin Feb 6, 2024 •

edited

Loading

rjmccall Feb 12, 2024 •

edited

Loading