tbaeder 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) { ---------------- aaron.ballman wrote: > 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.) I've pushed https://github.com/llvm/llvm-project/commit/11f5e5eb90c883d4b9ddba318e8fc57914b22ef3 As you can see, the problem also exists for static variables. I think this needs additional checks at interpretation time, or even new opcodes to check the declaration when the initializer is computed. So I'd rather have this in a separate followup patch. 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