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(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 struct X { static const int arr[]; };
> template constexpr int X::arr[] = {1, 2, 3};
> constexpr int k = X::arr[0];
>
> or:
>
> template struct X { static const double n; };
> template constexpr double X::n = 1;
> template struct Y {};
> Y<(int)X::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
