Skip to content

Better diagnostics when assertion fails in consteval #130458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,10 @@ Improvements to Clang's diagnostics

- An error is now emitted when OpenMP ``collapse`` and ``ordered`` clauses have an
argument larger than what can fit within a 64-bit integer.

- Explanatory note is printed when ``assert`` fails during evaluation of a
constant expression. Prior to this, the error inaccurately implied that assert
could not be used at all in a constant expression (#GH130458)

Improvements to Clang's time-trace
----------------------------------
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ def err_ice_too_large : Error<
"integer constant expression evaluates to value %0 that cannot be "
"represented in a %1-bit %select{signed|unsigned}2 integer type">;
def err_expr_not_string_literal : Error<"expression is not a string literal">;
def note_constexpr_assert_failed : Note<
"assertion failed during evaluation of constant expression">;

// Semantic analysis of constant literals.
def ext_predef_outside_function : Warning<
Expand Down
17 changes: 15 additions & 2 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5975,9 +5975,22 @@ static bool CheckConstexprFunction(EvalInfo &Info, SourceLocation CallLoc,
Definition->hasAttr<MSConstexprAttr>())))
return true;

if (Info.getLangOpts().CPlusPlus11) {
const FunctionDecl *DiagDecl = Definition ? Definition : Declaration;
const FunctionDecl *DiagDecl = Definition ? Definition : Declaration;
// Special note for the assert() macro, as the normal error message falsely
// implies we cannot use an assertion during constant evaluation.
if (CallLoc.isMacroID() && DiagDecl->getIdentifier()) {
// FIXME: Instead of checking for an implementation-defined function,
// check and evaluate the assert() macro.
Comment on lines +5982 to +5983
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you are not doing that in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea how to access the preprocessor from this method and I am too inexperienced to know where to start.
If you can give me a hint on where to look I would appreciate that. Do I just do a big refactor and pass it in as a parameter to the function? Or is there some method on an object I already have?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right, we would need access to Sema, which we currently don't. Good answer! (@Endilll !)
Given that, I am satisfied with the change as is, thanks

StringRef Name = DiagDecl->getName();
bool AssertFailed =
Name == "__assert_rtn" || Name == "__assert_fail" || Name == "_wassert";
if (AssertFailed) {
Info.FFDiag(CallLoc, diag::note_constexpr_assert_failed);
return false;
}
}

if (Info.getLangOpts().CPlusPlus11) {
// If this function is not constexpr because it is an inherited
// non-constexpr constructor, diagnose that directly.
auto *CD = dyn_cast<CXXConstructorDecl>(DiagDecl);
Expand Down
34 changes: 34 additions & 0 deletions clang/test/SemaCXX/consteval-assert.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// 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

#ifdef __ASSERT_FUNCTION
#undef __ASSERT_FUNCTION
#endif

#if defined(TEST_LINUX)
extern "C" void __assert_fail(const char*, const char*, unsigned, const char*);
#define assert(cond) \
((cond) ? (void)0 : __assert_fail(#cond, __FILE__, __LINE__, __func__))
#elif defined(TEST_DARWIN)
void __assert_rtn(const char *, const char *, int, const char *);
#define assert(cond) \
(__builtin_expect(!(cond), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #cond) : (void)0)
#elif defined(TEST_WINDOWS)
void /*__cdecl*/ _wassert(const wchar_t*, const wchar_t*, unsigned);
#define _CRT_WIDE_(s) L ## s
#define _CRT_WIDE(s) _CRT_WIDE_(s)
#define assert(cond) \
(void)((!!(cond)) || (_wassert(_CRT_WIDE(#cond), _CRT_WIDE(__FILE__), (unsigned)(__LINE__)), 0))
#endif

consteval int square(int x) {
int result = x * x;
assert(result == 42); // expected-note {{assertion failed during evaluation of constant expression}}
return result;
}

void test() {
auto val = square(2); // expected-note {{in call to 'square(2)'}} \
// expected-error {{call to consteval function 'square' is not a constant expression}}
}