[Sema] Fix a C++1z bug where initializer for static constexpr data member was not instantiated

[erik.pilkington]

Summary

This is a regression that only affects -std=c++1z, introduced in r273754. The following test case (thanks rsmith!) fails to compile:

template<int N> struct X {};

template<typename T> struct Y {
  static constexpr int s_v[] = {0};
  X<*s_v> x; // Clang incorrectly says: error, *s_v is not a constant expression
};

template struct Y<int>;

The problem is that the initializer for s_v (an implicitly inline variable in c++1z) is not instantiated. This patch requires that the initializer for an IncompleteArrayType is instantiated, which is the same rule as an initializer for an auto type.

Fixes PR28385.

Thanks!

[erik.pilkington]

Ping!

[rsmith]

lib/Sema/SemaTemplateInstantiateDecl.cpp

3861

I don't see why we would need the initializer right away in the IncompleteArrayType case. It seems instead that we should delay instantiation of the initializer until a complete type is required for the variable (Sema::RequireCompleteExprType handles this case) or it is used in a context that requires a definition (Sema::MarkVariableReferenced handles this case).

You can reproduce a related bug in C++11 mode like so:

template<typename T> struct X { static const int arr[]; };
template<typename T> constexpr int X<T>::arr[] = {1, 2, 3};
constexpr int k = X<int>::arr[0];

or:

template<typename T> struct X { static const double n; };
template<typename T> constexpr double X<T>::n = 1;
template<int> struct Y {};
Y<(int)X<int>::n> y;

It looks like the bug in this case is that VarDecl::isUsableInConstantExpressions (called from DoMarkVarDeclReferenced) is mishandling this case: once it's determined that the variable is of non-volatile const-qualified type, it needs to map back to the template instantiation pattern and check whether the most recent declaration of that is declared constexpr, since there might be a not-yet-instantiated redeclaration that adds the constexpr.

I'm not sure that's the same bug you're hitting here, though, since in this case we should be instantiating the constexpr specifier with the initial declaration, which should be enough to cause isUsableInConstantExpressions to return true. So the mystery is, why is DoMarkVarDeclReferenced not triggering instantiation?

[erik.pilkington]

lib/Sema/SemaTemplateInstantiateDecl.cpp

3861

Thanks for the pointer!

After looking some more into this, it looks like the problem isn't that the initializer for s_v isn't instantiated, its that it is instantiated after forming the DeclRefExpr to it, this means that the referring expression incorrectly has type IncompleteArrayType, which ExprConstant cannot handle. One approach is to patch up the offending DeclRefExpr to s_v in DoMarkVarDeclReferenced to have the correct type after instantiating s_v's initializer, which works fine.

To me, It seems cleaner to eagerly instantiate the initializer (as I did here) so that the DeclRefExpr has the correct type right off the bat. This seems very similar to the auto case (again, where the exact type isn't known) and is what is done in C++14 mode, FWIW.

If you know the right way to fix this (and have the time/inclination) you should definitely just do it. I would hate to stand in the way of a bug being fixed, especially one that is affecting so many users.

Thanks for helping!

[erik.pilkington]

Ping, sorry for the delay.

lib/Sema/SemaTemplateInstantiateDecl.cpp

                                                TSK_ImplicitInstantiation);
 
   // Forward the mangling number from the template to the instantiated decl.
   Context.setManglingNumber(NewVar, Context.getManglingNumber(OldVar));
   Context.setStaticLocalNumber(NewVar, Context.getStaticLocalNumber(OldVar));
 
   // Delay instantiation of the initializer for variable templates or inline
   // static data members until a definition of the variable is needed. We need
-  // it right away if the type contains 'auto'.
+  // it right away if the type contains 'auto' or is an IncompleteArrayType.
   if ((!isa<VarTemplateSpecializationDecl>(NewVar) &&
        !InstantiatingVarTemplate &&
        !(OldVar->isInline() && OldVar->isThisDeclarationADefinition())) ||
-      NewVar->getType()->isUndeducedType())
+      NewVar->getType()->isUndeducedType() ||
+      NewVar->getType()->isIncompleteArrayType())
     InstantiateVariableInitializer(NewVar, OldVar, TemplateArgs);
 
   // Diagnose unused local variables with dependent types, where the diagnostic
   // will have been deferred.
   if (!NewVar->isInvalidDecl() &&
       NewVar->getDeclContext()->isFunctionOrMethod() &&
       OldVar->getType()->isDependentType())
     DiagnoseUnusedDecl(NewVar);

test/CXX/temp/temp.spec/temp.inst/p1.cpp

int g() {
     enum E {
       e = T::error // expected-error {{has no members}}
     };
     return E::e;
   }
   int test2 = g<int>(); // expected-note {{here}}
 }
 
+namespace PR28385 {
+template <int N> struct X {};
+
+template <typename T> struct Y {
+  static constexpr int s_v[] = {0};
+  X<*s_v> x;
+};
+
+template struct Y<int>;
+}
+
 // FIXME:
 //- - member anonymous unions