[clang] Migrate DR tests to static_assert
#88611
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang Author: Vlad Serebrennikov (Endilll) ChangesThis patch touches a number of tests that run in C++98 mode that have been using array size as a context that requires a constant expression, replacing it with a This patch also showcases that DR tests would benefit from olding in constant expression in C++98 mode, but I'm not sure it's even on the table. If it is, I'd be happy to prepare a PR for that, and rebase this PR on top of it. CC @AaronBallman Full diff: https://github.com/llvm/llvm-project/pull/88611.diff 6 Files Affected:
diff --git a/clang/test/CXX/drs/dr0xx.cpp b/clang/test/CXX/drs/dr0xx.cpp
index a304862885c640..c30d9710e8d00a 100644
--- a/clang/test/CXX/drs/dr0xx.cpp
+++ b/clang/test/CXX/drs/dr0xx.cpp
@@ -5,6 +5,11 @@
// RUN: %clang_cc1 -std=c++20 %s -verify=expected,since-cxx11,since-cxx17 -fexceptions -fcxx-exceptions -pedantic-errors -triple %itanium_abi_triple
// RUN: %clang_cc1 -std=c++23 %s -verify=expected,since-cxx11,since-cxx17 -fexceptions -fcxx-exceptions -pedantic-errors -triple %itanium_abi_triple
+#if __cplusplus == 199711L
+#define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
+// cxx98-error@-1 {{variadic macros are a C99 feature}}
+#endif
+
namespace cwg1 { // cwg1: no
namespace X { extern "C" void cwg1_f(int a = 1); }
namespace Y { extern "C" void cwg1_f(int a = 1); }
@@ -1163,10 +1168,9 @@ namespace cwg75 { // cwg75: yes
namespace cwg76 { // cwg76: yes
const volatile int n = 1;
- int arr[n]; // #cwg76-vla
- // expected-error@#cwg76-vla {{variable length arrays in C++ are a Clang extension}}
- // expected-note@#cwg76-vla {{read of volatile-qualified type 'const volatile int' is not allowed in a constant expression}}
- // expected-error@#cwg76-vla {{variable length array declaration not allowed at file scope}}
+ static_assert(n, "");
+ // expected-error@-1 {{static assertion expression is not an integral constant expression}}
+ // expected-note@-2 {{read of volatile-qualified type 'const volatile int' is not allowed in a constant expression}}
}
namespace cwg77 { // cwg77: yes
diff --git a/clang/test/CXX/drs/dr16xx.cpp b/clang/test/CXX/drs/dr16xx.cpp
index 6d7bb7619f8b8b..cf6b45ceabf2cc 100644
--- a/clang/test/CXX/drs/dr16xx.cpp
+++ b/clang/test/CXX/drs/dr16xx.cpp
@@ -153,10 +153,9 @@ namespace cwg1645 { // cwg1645: 3.9
namespace cwg1652 { // cwg1652: 3.6
int a, b;
- int arr[&a + 1 == &b ? 1 : 2];
- // expected-error@-1 {{variable length arrays in C++ are a Clang extension}}
+ static_assert(&a + 1 == &b, "");
+ // expected-error@-1 {{static assertion expression is not an integral constant expression}}
// expected-note@-2 {{comparison against pointer '&a + 1' that points past the end of a complete object has unspecified value}}
- // expected-error@-3 {{variable length array declaration not allowed at file scope}}
}
namespace cwg1653 { // cwg1653: 4 c++17
diff --git a/clang/test/CXX/drs/dr1xx.cpp b/clang/test/CXX/drs/dr1xx.cpp
index 5b497dda047d6a..276ac0c11a0e77 100644
--- a/clang/test/CXX/drs/dr1xx.cpp
+++ b/clang/test/CXX/drs/dr1xx.cpp
@@ -5,6 +5,17 @@
// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11,since-cxx17 -fexceptions -fcxx-exceptions -pedantic-errors
// RUN: %clang_cc1 -std=c++23 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11,since-cxx17 -fexceptions -fcxx-exceptions -pedantic-errors
+#if __cplusplus == 199711L
+#define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
+// cxx98-error@-1 {{variadic macros are a C99 feature}}
+#endif
+
+#if __cplusplus == 199711L
+#define __enable_constant_folding(x) (__builtin_constant_p(x) ? (x) : (x))
+#else
+#define __enable_constant_folding
+#endif
+
namespace cwg100 { // cwg100: yes
template<const char (*)[4]> struct A {}; // #cwg100-A
template<const char (&)[4]> struct B {}; // #cwg100-B
@@ -745,13 +756,7 @@ namespace cwg148 { // cwg148: yes
namespace cwg151 { // cwg151: 3.1
struct X {};
typedef int X::*p;
-#if __cplusplus < 201103L
-#define fold(x) (__builtin_constant_p(0) ? (x) : (x))
-#else
-#define fold
-#endif
- int check[fold(p() == 0) ? 1 : -1];
-#undef fold
+ static_assert(__enable_constant_folding(p() == 0), "");
}
namespace cwg152 { // cwg152: yes
diff --git a/clang/test/CXX/drs/dr2xx.cpp b/clang/test/CXX/drs/dr2xx.cpp
index e655e7226d51d6..2f1d18008d04db 100644
--- a/clang/test/CXX/drs/dr2xx.cpp
+++ b/clang/test/CXX/drs/dr2xx.cpp
@@ -10,10 +10,15 @@
typedef __SIZE_TYPE__ size_t;
// cxx98-error@-1 0-1 {{'long long' is a C++11 extension}}
-#if __cplusplus < 201103L
-#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+#if __cplusplus == 199711L
+#define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
+// cxx98-error@-1 {{variadic macros are a C99 feature}}
+#endif
+
+#if __cplusplus == 199711L
+#define __enable_constant_folding(x) (__builtin_constant_p(x) ? (x) : (x))
#else
-#define fold
+#define __enable_constant_folding
#endif
namespace cwg200 { // cwg200: dup 214
@@ -31,7 +36,7 @@ namespace cwg200 { // cwg200: dup 214
namespace cwg202 { // cwg202: 3.1
template<typename T> T f();
template<int (*g)()> struct X {
- int arr[fold(g == &f<int>) ? 1 : -1];
+ static_assert(__enable_constant_folding(g == &f<int>), "");
};
template struct X<f>;
}
@@ -1024,7 +1029,7 @@ namespace cwg275 { // cwg275: no
namespace cwg277 { // cwg277: 3.1
typedef int *intp;
int *p = intp();
- int a[fold(intp() ? -1 : 1)];
+ static_assert(__enable_constant_folding(intp() ? -1 : 1), "");
}
namespace cwg280 { // cwg280: 2.9
diff --git a/clang/test/CXX/drs/dr3xx.cpp b/clang/test/CXX/drs/dr3xx.cpp
index 6d1c6958ac8eb6..4a9682c335915e 100644
--- a/clang/test/CXX/drs/dr3xx.cpp
+++ b/clang/test/CXX/drs/dr3xx.cpp
@@ -5,6 +5,17 @@
// RUN: %clang_cc1 -std=c++11 -verify=expected,cxx98-14,cxx98-17,cxx98-20,cxx11-14,since-cxx11 -triple %itanium_abi_triple %s -fexceptions -fcxx-exceptions -pedantic-errors
// RUN: %clang_cc1 -std=c++98 -verify=expected,cxx98-14,cxx98-17,cxx98-20,cxx98 -triple %itanium_abi_triple %s -fexceptions -fcxx-exceptions -pedantic-errors
+#if __cplusplus == 199711L
+#define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
+// cxx98-error@-1 {{variadic macros are a C99 feature}}
+#endif
+
+#if __cplusplus == 199711L
+#define __enable_constant_folding(x) (__builtin_constant_p(x) ? (x) : (x))
+#else
+#define __enable_constant_folding
+#endif
+
namespace cwg300 { // cwg300: yes
template<typename R, typename A> void f(R (&)(A)) {}
int g(int);
@@ -1099,21 +1110,14 @@ namespace cwg364 { // cwg364: yes
#endif
namespace cwg367 { // cwg367: yes
- // FIXME: These diagnostics are terrible. Don't diagnose an ill-formed global
- // array as being a VLA!
- int a[true ? throw 0 : 4];
- // expected-error@-1 {{variable length arrays in C++ are a Clang extension}}
- // expected-error@-2 {{variable length array declaration not allowed at file scope}}
- int b[true ? 4 : throw 0];
- // cxx98-error@-1 {{variable length arrays in C++ are a Clang extension}}
- // cxx98-error@-2 {{variable length array folded to constant array as an extension}}
- int c[true ? *new int : 4];
- // expected-error@-1 {{variable length arrays in C++ are a Clang extension}}
+ static_assert(__enable_constant_folding(true ? throw 0 : 4), "");
+ // expected-error@-1 {{expression is not an integral constant expression}}
+ static_assert(__enable_constant_folding(true ? 4 : throw 0), "");
+ static_assert(__enable_constant_folding(true ? *new int : 4), "");
+ // expected-error@-1 {{expression is not an integral constant expression}}
// expected-note@-2 {{read of uninitialized object is not allowed in a constant expression}}
- // expected-error@-3 {{variable length array declaration not allowed at file scope}}
- int d[true ? 4 : *new int];
- // cxx98-error@-1 {{variable length arrays in C++ are a Clang extension}}
- // cxx98-error@-2 {{variable length array folded to constant array as an extension}}
+ static_assert(__enable_constant_folding(true ? 4 : *new int), "");
+
}
namespace cwg368 { // cwg368: 3.6
diff --git a/clang/test/CXX/drs/dr4xx.cpp b/clang/test/CXX/drs/dr4xx.cpp
index 611b791470785d..b8d754da358191 100644
--- a/clang/test/CXX/drs/dr4xx.cpp
+++ b/clang/test/CXX/drs/dr4xx.cpp
@@ -6,6 +6,11 @@
// RUN: env ASAN_OPTIONS=detect_stack_use_after_return=0 %clang_cc1 -std=c++23 %s -verify=expected,since-cxx20,since-cxx17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
// RUN: env ASAN_OPTIONS=detect_stack_use_after_return=0 %clang_cc1 -std=c++2c %s -verify=expected,since-cxx20,since-cxx17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+#if __cplusplus == 199711L
+#define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
+// cxx98-error@-1 {{variadic macros are a C99 feature}}
+#endif
+
// FIXME: __SIZE_TYPE__ expands to 'long long' on some targets.
__extension__ typedef __SIZE_TYPE__ size_t;
@@ -842,11 +847,10 @@ namespace cwg451 { // cwg451: yes
// expected-warning@-1 {{division by zero is undefined}}
const int b = 1 / 0; // #cwg451-b
// expected-warning@-1 {{division by zero is undefined}}
- int arr[b]; // #cwg451-arr
- // expected-error@-1 {{variable length arrays in C++ are a Clang extension}}
+ static_assert(b, "");
+ // expected-error@-1 {{expression is not an integral constant expression}}
// expected-note@-2 {{initializer of 'b' is not a constant expression}}
// expected-note@#cwg451-b {{declared here}}
- // expected-error@#cwg451-arr {{variable length array declaration not allowed at file scope}}
}
namespace cwg452 { // cwg452: yes
@@ -876,11 +880,10 @@ namespace cwg456 { // cwg456: yes
namespace cwg457 { // cwg457: yes
const int a = 1;
const volatile int b = 1;
- int ax[a];
- int bx[b];
- // expected-error@-1 {{variable length arrays in C++ are a Clang extension}}
+ static_assert(a, "");
+ static_assert(b, "");
+ // expected-error@-1 {{expression is not an integral constant expression}}
// expected-note@-2 {{read of volatile-qualified type 'const volatile int' is not allowed in a constant expression}}
- // expected-error@-3 {{variable length array declaration not allowed at file scope}}
enum E {
ea = a,
@@ -1366,11 +1369,10 @@ namespace cwg486 { // cwg486: yes
namespace cwg487 { // cwg487: yes
enum E { e };
int operator+(int, E); // #cwg487-operator-plus
- int i[4 + e]; // #cwg487-i
- // expected-error@-1 {{variable length arrays in C++ are a Clang extension}}
+ static_assert(4 + e, "");
+ // expected-error@-1 {{expression is not an integral constant expression}}
// since-cxx11-note@-2 {{non-constexpr function 'operator+' cannot be used in a constant expression}}
// since-cxx11-note@#cwg487-operator-plus {{declared here}}
- // expected-error@#cwg487-i {{variable length array declaration not allowed at file scope}}
}
namespace cwg488 { // cwg488: yes c++11
|
|
This PR removes a FIXME since it doesn't make sense anymore in that particular context. I created issue #88608 out of it to make sure we don't lose track of it. |
Sirraide
reviewed
Apr 13, 2024
Comment on lines
+8
to
+17
| #if __cplusplus == 199711L | ||
| #define static_assert(...) __extension__ _Static_assert(__VA_ARGS__) | ||
| // cxx98-error@-1 {{variadic macros are a C99 feature}} | ||
| #endif | ||
|
|
||
| #if __cplusplus == 199711L | ||
| #define __enable_constant_folding(x) (__builtin_constant_p(x) ? (x) : (x)) | ||
| #else | ||
| #define __enable_constant_folding | ||
| #endif |
There was a problem hiding this comment.
I don’t think we normally do that too often for tests, but this is the third file or so that I’ve seen so far that uses these macros, so I’d maybe suggest moving them into a header that the tests can include (especially considering you said you’re going to refactor more tests than just these) if that doesn’t cause any problems.
There was a problem hiding this comment.
You're correct that having a very thin standard/backporting library would be beneficial. There have been efforts last year to improve testing infrastructure, and this particular detail was a part of the plan, but those efforts have been stalled for months. I'd keep things as is until the time comes.
I've put up an RFC a year ago if you're interested in the context: https://discourse.llvm.org/t/rfc-opt-in-way-to-make-verifydiagnosticconsumer-slightly-less-strict/70747
@Sirraide I didn't say that originally, but you reminded me that I indeed forgot about all the uses of arrays that don't trigger diagnostics. I went over all of them, and decided to include them in this patch instead of doing a follow-up NFC commit, since it's not too much of them actually. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch touches a number of tests that run in C++98 mode that have been using array size as a context that requires a constant expression, replacing it with a
static_assertbackported via a macro. This reduces noise in expected directives that comes from diagnostics around VLAs.This patch also showcases that DR tests would benefit from folding in constant expressions in C++98 mode, but I'm not sure it's even on the table. If it is, I'd be happy to prepare a PR for that, and rebase this PR on top of it.
CC @AaronBallman