[clang][bytecode] Diagnose failed constexpr assertions differently #140000
Conversation
Adjust to the new way the ast walker is doing it.
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesAdjust to the new way the ast walker is doing it. Full diff: https://github.com/llvm/llvm-project/pull/140000.diff 2 Files Affected:
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index bc860185fea21..ab4bd96b4f993 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -851,6 +851,19 @@ bool CheckCallable(InterpState &S, CodePtr OpPC, const Function *F) {
if (F->isLambdaStaticInvoker())
return true;
+ // Diagnose failed assertions specially.
+ if (S.Current->getLocation(OpPC).isMacroID() &&
+ F->getDecl()->getIdentifier()) {
+ StringRef Name = F->getDecl()->getName();
+ bool AssertFailed =
+ Name == "__assert_rtn" || Name == "__assert_fail" || Name == "_wassert";
+ if (AssertFailed) {
+ S.FFDiag(S.Current->getLocation(OpPC),
+ diag::note_constexpr_assert_failed);
+ return false;
+ }
+ }
+
if (S.getLangOpts().CPlusPlus11) {
const FunctionDecl *DiagDecl = F->getDecl();
diff --git a/clang/test/SemaCXX/consteval-assert.cpp b/clang/test/SemaCXX/consteval-assert.cpp
index b54a38ff26105..8f0b9046518ee 100644
--- a/clang/test/SemaCXX/consteval-assert.cpp
+++ b/clang/test/SemaCXX/consteval-assert.cpp
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -std=c++23 -verify=expected,cxx20_plus -DTEST_LINUX %s
-// RUN: %clang_cc1 -std=c++23 -verify=expected,cxx20_plus -DTEST_WINDOWS %s
-// RUN: %clang_cc1 -std=c++23 -verify=expected,cxx20_plus -DTEST_DARWIN %s
+// RUN: %clang_cc1 -std=c++23 -verify -DTEST_LINUX %s
+// RUN: %clang_cc1 -std=c++23 -verify -DTEST_WINDOWS %s
+// RUN: %clang_cc1 -std=c++23 -verify -DTEST_DARWIN %s
+
+// RUN: %clang_cc1 -std=c++23 -verify -DTEST_LINUX %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -std=c++23 -verify -DTEST_WINDOWS %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -std=c++23 -verify -DTEST_DARWIN %s -fexperimental-new-constant-interpreter
#ifdef __ASSERT_FUNCTION
#undef __ASSERT_FUNCTION
|
| F->getDecl()->getIdentifier()) { | ||
| StringRef Name = F->getDecl()->getName(); | ||
| bool AssertFailed = | ||
| Name == "__assert_rtn" || Name == "__assert_fail" || Name == "_wassert"; |
There was a problem hiding this comment.
Can you add a FIXME comment with a link to a GitHub issue? Ideally, detection of an assertion failure would evaluate the assert macro instead of looking at the implementation specific function. If this is done in ExprConstant.cpp it should probably be done here as well.
There was a problem hiding this comment.
Link to what github issue?
I removed the fixme comment because it's already in ExprConstant.cpp and fixme comments like that don't tend to get fixed anyway.
There was a problem hiding this comment.
I actually want to fix it at some point since I wrote the original code. My skillset is bad though which is why I couldn't figure it out. I'll make the GitHub issue and assign it to myself. It's more of a reminder to me that I have to fix it here as well.
Adjust to the new way the ast walker is doing it.