[PATCH] D156453: [clang][Interp] Create only globals when initializing a global variable

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1213
   std::optional<PrimType> SubExprT = classify(SubExpr);
-  if (E->getStorageDuration() == SD_Static) {
+  bool IsStatic = E->getStorageDuration() == SD_Static;
+  if (GlobalDecl || IsStatic) {
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > Should we be looking at the TLS kind of the extended declaration? (`CheckCompleteVariableDeclaration()` is using `VarDecl::getTLSKind() == VarDecl::TLS_Static`)
> > 
> > Would something along these lines work instead?
> > ```
> > bool EmitGlobalTemp = E->getStorageDuration() == SD_Static;
> > if (!EmitGlobalTemp) {
> >   if (const LifetimeExtendedTemporaryDecl *LETD = E->getLifetimeExtendedTemporaryDecl()) {
> >     if (const auto *VD = dyn_cast_if_present<VarDecl>(LETD->getExtendingDecl()) {
> >       EmitGlobalTemp= VD->getTLSKind() == VarDecl::TLS_Static;
> >     }
> >   }
> > }
> > ```
> That code definitely works for the current `AST/Interp/` tests, but we don't have tests for thread local stuff in there right now.
Hmm, I think we'll need those tests: https://eel.is/c++draft/expr.const#5.2

That seems to be the only mention about thread local storage duration for constant expressions in C++, so it might make sense to tackle that as part of this change?

(I worry that we'll forget to come back to this detail later, basically. So either we should have failing test coverage showing we need a fix, an issue in GitHub so we know to come back to it, etc. or just do the work up front given that it's closely related.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156453/new/

https://reviews.llvm.org/D156453