mehdi_amini added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2374
+ llvm::Constant *Init = nullptr;
+ if (D && D->isConstexpr() && !D->isInline() && !D->hasAttr<DLLImportAttr>())
{
+ const VarDecl *InitDecl;
----------------
rsmith wrote:
> Applying this for only `constexpr` variables seems unnecessarily
> conservative; it seems we could equally do this for any variable that is
> `const` and has no `mutable` fields (though we'd need to check for
> `EmitConstantInit` failing in the general case).
OK I will improve, and include a test with a struct with a mutable field.
================
Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:1
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu |
FileCheck %s
+
----------------
rsmith wrote:
> Please also test what happens in C++1z mode, particularly as static constexpr
> data members are implicitly `inline` there.
This is already covered by ```clang/test/CodeGenCXX/cxx1z-inline-variables.cpp
``` (I actually broke this test during development because I didn't know about
inline variable). But I can also add coverage for it here if you'd like.
================
Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:15
+// CHECK: @_ZL4BAR3 = available_externally constant i32 44,
+static constexpr int BAR3 = 44;
+
----------------
rsmith wrote:
> mehdi_amini wrote:
> > Looks like I have a bug here, this should be an internal.
> I would imagine that we skip promotion of declaration to definition in this
> case if we already have a definition.
>
> To that end, please add a testcase like this:
>
> ```
> struct Bar {
> static constexpr int BAZ = 42;
> };
> auto *use = &Bar::BAZ;
> const int Bar::BAZ;
> ```
>
> ... to make sure that we convert the definition of `Bar::BAZ` from
> `available_externally` to a strong definition (I think we should end up with
> `weak_odr` here).
`weak_odr` in C++17 because it is an inline variable, but I expect a strong
definition in c++11.
I'll add this, this is a good test-case!
https://reviews.llvm.org/D34992
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
