[PATCH] Fix IRGen for referencing a static local before emitting its decl

Reid Kleckner rnk at google.com
Fri Sep 5 15:20:42 PDT 2014


This is turning out to be really, really difficult.

================
Comment at: lib/CodeGen/CGDecl.cpp:174
@@ -173,4 +173,3 @@
 
-llvm::Constant *
-CodeGenFunction::CreateStaticVarDecl(const VarDecl &D,
-                                     llvm::GlobalValue::LinkageTypes Linkage) {
+llvm::Constant *CodeGenFunction::getOrCreateStaticVarDecl(
+    const VarDecl &D, llvm::GlobalValue::LinkageTypes Linkage) {
----------------
rsmith wrote:
> Does it still make sense for this to be on `CodeGenFunction` since it can now be called while emitting the "wrong" function? It seems to risk using local state of the `CodeGenFunction` object, which would be bad.
It seems there's two uses of local state:
- CGF affects name mangling for static locals in non-C++ languages, but in those languages, the name is presumably not significant
- CGF matters to CGExprConstant, in particular for taking the address of a label

================
Comment at: lib/CodeGen/CGDecl.cpp:191
@@ -184,3 +190,3 @@
   else
     Name = GetStaticDeclName(*this, D);
 
----------------
rsmith wrote:
> Case in point: this will presumably produce the wrong mangled name if the static local's emission is triggered by a function other than the containing one.
No, in C++ mode it just mangles the VarDecl, which has all the info needed.

================
Comment at: lib/CodeGen/CGDecl.cpp:199
@@ -192,3 +198,3 @@
                              Ty.isConstant(getContext()), Linkage,
                              CGM.EmitNullConstant(D.getType()), Name, nullptr,
                              llvm::GlobalVariable::NotThreadLocal,
----------------
rsmith wrote:
> This doesn't look right. If the static local has a constant initializer, you need to emit it here, not as part of emitting the surrounding function. Given:
> 
>   static auto f() {
>     static int n = 1;
>     struct S { int &operator()() { return n; } };
>     return S();
>   }
> 
>   int main() { return decltype(f())()(); }
> 
> ... `main` should return 1, and I think with this patch it returns 0.
OK. This can be done, but it has considerable complexity because the initializer can reference itself circularly, apparently.

http://reviews.llvm.org/D4787






More information about the cfe-commits mailing list