[PATCH] D145859: [Clang][CodeGen] Fix linkage of template parameter objects

[Richard Smith - zygoloid via Phabricator via llvm-commits]

rsmith added a comment.

Looks good. If you want to look at the visibility side of this separately I think that's fine (I only called it out here because we often consider them at the same time).



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3241-3246
+  llvm::GlobalValue::LinkageTypes Linkage =
+      isExternallyVisible(TPO->getLinkageAndVisibility().getLinkage())
+          ? llvm::GlobalValue::LinkOnceODRLinkage
+          : llvm::GlobalValue::InternalLinkage;
+  auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(),
+                                      /*isConstant=*/true, Linkage, Init, Name);
----------------
I think we should be taking the visibility into account in some way too. For example, if the type of the template parameter object involves a type with hidden visibility, then the template parameter object itself should have hidden visibility.

Can we call `setGVProperties(GV, TPO);` here? It looks like that would do the right thing for visibility.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145859/new/

https://reviews.llvm.org/D145859