[PATCH] D50968: [AST] make a static local variable in a hidden inlined fuction visible

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lib/AST/Decl.cpp:1272
+      assert(cast<VarDecl>(D)->isStaticLocal());
+      return LinkageInfo(VisibleNoLinkage, DefaultVisibility, false);
+    }
----------------
jsji wrote:
> Is it overreaction here if we always return "DefaultVisibility"? Is it possible that the visibility of this static was already hidden without -fvisibility-inlines-hidden?
> 
> Maybe we should consider avoid calling mergeVisibility for local static only?
I think this incorrectly gives `x` default visibility when the hidden visibility attribute is explicit and compiling with -fvisibility-inlines-hidden, as in the example below:
```
inline int *__attribute__((visibility("hidden"))) f() {
  static int x;
  return &x;
}
```

You should add this test and confirm that we behave as GCC does.

You should also consider cases where the attribute is explicitly put on the variable itself and make sure that is handled.


================
Comment at: lib/AST/Decl.cpp:1275
+
     LV = getLVForDecl(FD, computation);
   }
----------------
I think the right way to do this is to compute the linkage & visibility of FD, as is done here, check if the visibility is explicit, and if not, under the same conditions that you have above, upgrade the visibility to default.


https://reviews.llvm.org/D50968