[PATCH] D34992: Emit static constexpr member as available_externally definition

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

> 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:

Added!



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2448-2451
+        if (InitExpr) {
+          GV->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage);
+          GV->setInitializer(EmitConstantInit(*InitDecl));
+        }
----------------
rsmith wrote:
> 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;
> ```
But this wouldn't be an "available_externally". We need the initializer in the class definition.

I tried multiple variant but couldn't get one that compile:

```
struct B {
  static bool check() {
    if (B::n == 0) return false;
    return true;
  }
  // error: in-class initializer for static data member is not a constant expression
  static const int n = (B::check() || true) ? 1 : 2;
};
```

```
struct B {
  static constexpr bool check() {
// "error: constexpr function never produces a constant expression"
    return  (B::n == 0 || true) ? false : true;
  }
//"note: initializer of 'n' is not a constant expression"
  static constexpr int n = (B::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()) {
----------------
rsmith wrote:
> Do we need to special-case this? Such declarations are definitions.
We don't! Simplified after rebasing and adapting after John's changes.


Repository:
  rL LLVM

https://reviews.llvm.org/D34992