rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

I'd like to also see a testcase for the situation where we trigger the emission 
of a declaration with an `available_externally` definition and then find we 
need to promote it to a "full" definition:

  struct A {
    static const int Foo = 123;
  };
  int *p = &A::Foo; // emit available_externally
  const int A::Foo; // convert to full definition



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2448-2451
+        if (InitExpr) {
+          GV->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage);
+          GV->setInitializer(EmitConstantInit(*InitDecl));
+        }
----------------
mehdi_amini wrote:
> rsmith wrote:
> > In order for this transformation to be correct, you need to know that the 
> > variable has static initialization, which means that it needs to formally 
> > have a constant initializer. You can use `D->isInitKnownICE()` to check 
> > this.
> Done! But couldn't find how to express it as a test-case though.
You'd need to use an initializer that we can constant-fold, such as:

```
struct A {
  static const int n;
};
bool check() {
  assert(A::n == 0 && "already initialized!");
  return true;
}
const int A::n = (check() || true) ? 1 : 2;
```


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2445
+        !D->hasDefinition() && D->hasInit() &&
+        /* C++17 static constexpr are inlined */ !D->isInline() &&
+        !D->hasAttr<DLLImportAttr>() && D->isInitKnownICE()) {
----------------
Do we need to special-case this? Such declarations are definitions.


https://reviews.llvm.org/D34992



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

Reply via email to