-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Description
[[msvc::constexpr]]
support was added in #71300.
However, it was lacking support of [[msvc::constexpr]]
constructors,
because I was not sure how to support them with reasonable changes.
Consider the following test case (from clang/test/SemaCXX/ms-constexpr.cpp
):
struct S2 {
[[msvc::constexpr]] S2() {}
[[msvc::constexpr]] bool value() { return true; }
static constexpr bool check() { [[msvc::constexpr]] return S2{}.value(); }
};
static_assert(S2::check());
It's a valid code for MSVC: https://godbolt.org/z/znnaonEhM
However supporting this code in Clang seemed to be difficult:
S2
fails checks of "literal type" in this callstack:
We have information about CanEvalMSConstexpr
only in CheckLiteralType
. Two obvious ugly solutions were:
- Copy all checks from
clang::CXXRecordDecl::isLiteral
toCheckLiteralType
- two places shall be maintained. - Propagate
CanEvalMSConstexpr
down toclang::CXXRecordDecl::isLiteral
. We could add bool args for bothclang::CXXRecordDecl::isLiteral
andclang::Type::isLiteralType
. But I don't think it's reasonable for supporting rare vendor-specific attribute. Or is it?
I found another variant of the implementation: let ASTContext
know about CanEvalMSConstexpr
, and pass const ASTContext&
to clang::CXXRecordDecl::isLiteral
.
In long term, if clang ever switches to new constexpr analyzer (-fexperimental-new-constant-interpreter
), then (as far as I understand) CanEvalMSConstexpr
could be moved from ASTContext
to clang::interp::Context
. Then clang::CXXRecordDecl::isLiteral
could accept this context instead of ASTContext
.