llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.15k stars 12.03k forks source link

constexpr pointer member with non-constexpr out-of-class definition not allowed in a constant expression #57894

Open andreyv opened 2 years ago

andreyv commented 2 years ago

Consider the following code, reduced from Boost.Asio:

struct tracked_t {
  static constexpr void *static_query_v = 0;
};
void * const tracked_t::static_query_v;
static constexpr auto r = tracked_t::static_query_v;

A constexpr variable can have a non-constexpr declaration. Additionally, prior to C++17 the in-class declaration here is not a definition, and the definition itself is out-of-class. (C++17 changed this, see appendix D.1 [depr.static_constexpr].)

In C++11 or C++14 mode, Clang gives

x.cpp:6:23: error: constexpr variable 'r' must be initialized by a constant expression
static constexpr auto r = tracked_t::static_query_v;
                      ^   ~~~~~~~~~~~~~~~~~~~~~~~~~
x.cpp:6:27: note: read of non-constexpr variable 'static_query_v' is not allowed in a constant expression
static constexpr auto r = tracked_t::static_query_v;
                          ^
x.cpp:5:25: note: declared here
void * const tracked_t::static_query_v;
                        ^
1 error generated.

on this sample. This is because Clang looks at the definition of static_query_v, which is out-of-class and does not have the constexpr specifier, even though the variable itself is constexpr.

The relevant code is here: https://github.com/llvm/llvm-project/blob/b73d2c8c720a8c8e6e73b11be4e27afa6cb75bdf/clang/lib/AST/ExprConstant.cpp#L4054-L4058 It retrieves the definition, if one is available, and later checks VD->isConstexpr() on it.

Note that the same sample, but with int instead of a pointer type is accepted, because the code takes the BaseType->isIntegralOrEnumerationType() branch later.

andreyv commented 2 years ago

This patch appears to fix the problem by handling the very specific case:

diff -Nru a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
--- a/clang/lib/AST/ExprConstant.cpp    2022-09-20 09:05:50.000000000 +0300
+++ b/clang/lib/AST/ExprConstant.cpp    2022-09-21 14:52:29.985976264 +0300
@@ -4081,6 +4081,12 @@
         return CompleteObject();
       } else if (VD->isConstexpr()) {
         // OK, we can read this variable.
+      } else if (VD->isStaticDataMember() &&
+                 VD->getCanonicalDecl()->isConstexpr()) {
+        // OK, the out-of-class definition of the variable is not constexpr,
+        // but the in-class declaration is constexpr and must be initialized.
+        assert(!Info.getLangOpts().CPlusPlus17 &&
+               "VD should have already been constexpr");
       } else if (BaseType->isIntegralOrEnumerationType()) {
         if (!IsConstant) {
           if (!IsAccess)

but I'm not sure if this is the correct approach.

Additionally, it would be better to use VD->getInitializingDeclaration() here, since this one declaration must have constexpr, if any. But, at least on this sample, the function seems to return the same out-of-class declaration, which contradicts its description: https://github.com/llvm/llvm-project/blob/b73d2c8c720a8c8e6e73b11be4e27afa6cb75bdf/clang/include/clang/AST/Decl.h#L1293-L1296

llvmbot commented 2 years ago

@llvm/issue-subscribers-clang-frontend