erik.pilkington added inline comments. ================ Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3861 @@ -3860,3 +3860,3 @@ // 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) && ---------------- rsmith wrote: > 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? 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! https://reviews.llvm.org/D22053 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits