akhuang added a subscriber: rsmith.
akhuang added inline comments.
================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1421-1425
+ else if (auto *TemplateDecl = Var->getInstantiatedFromStaticDataMember()) {
+ // Inline static data members might not have an initialization.
+ if (TemplateDecl->getInit())
+ Value = TemplateDecl->evaluateValue();
+ }
----------------
dblaikie wrote:
> I'd suspect this might not be the most general solution - because it's
> retrieving the value from the original template static member. Two issues
> come to mind:
> 1) If the initialization is dependent or otherwise not knowable at
> compile-time, does this code at least not crash (I assume "evaluateValue"
> returns nullptr if the value can't be evaluated for whatever reason)
> 2) If the initialization is dependent, it might still be knowable, eg:
>
> ```
> template<typename T>
> struct static_decl_templ {
> static constexpr int static_constexpr_decl_templ_var = sizeof(T);
> };
> ```
>
> I'm guessing this patch as-is doesn't provide a value in this case, but it
> should be achievable, I would guess/hope/imagine - but don't really know
> exactly how to do that off-hand. (@zygoloid would know - but maybe a bit of
> looking around will find it if/before he has a chance to weigh in here)
>
>
@rsmith
Actually, can we just instantiate the initializer for static constexpr data
members?
currently
[[https://github.com/llvm/llvm-project/blob/683b308c07bf827255fe1403056413f790e03729/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L5062|
it delays creating the initializer for inline static data members ]]; can we
not do that if it's a constexpr?
================
Comment at: clang/test/CodeGenCXX/debug-info-static-member.cpp:140-144
+#if __cplusplus >= 201103L
+ static constexpr int static_constexpr_decl_templ_var = 8;
+#else
+ static const int static_constexpr_decl_templ_var = 8;
+#endif
----------------
dblaikie wrote:
> I guess the non-11 version of this variable is intended as a stub to make the
> test across different language levels simpler?
>
> I think it might be better to make that more explicit - rather than having a
> variable with "constexpr" in the name that isn't actually constexpr. I'd say
> only add the variable under that macro condition - but change the test to use
> multiple --check-prefixes, and the check for this variable to use a CPP11
> prefix or something like that.
>
> Hmm, I'm slightly confused now that I Think about it - it looks like the
> macro is about testing if the version is >= 11. But the change also adds a
> test for C++17? Is C++17 likely to be any different than C++11 for any of
> this? (or C++20 for that matter) if it's not any different, I'd probably
> leave the 17 case out & expect the 11 case is sufficient coverage.
Yeah, it was intended as a stub, but I can remove that part.
I added c++17 because it appears that in c++17 (and for MS targets) we make
constexpr data members implicitly inline, which is why the initializer is not
being instantiated.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89286/new/
https://reviews.llvm.org/D89286
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
