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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to