[PATCH] D49109: Borrow visibility from __fundamental_type_info for generated fundamental type infos

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a subscriber: cfe-commits.
pcc added inline comments.


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3215
   // Emit the standard library with external linkage.
   llvm::GlobalVariable::LinkageTypes Linkage;
   if (IsStdLib)
----------------
`IsStdLib` will always be false at this point because of the if statement on line 3211 so I think you can change this to just `llvm::GlobalVariable::LinkageTypes Linkage = getTypeInfoLinkage(CGM, Ty);` and fold the call to `IsStandardLibraryRTTIDescriptor(Ty)` into the if statement.


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3239-3241
+    } else if (RD && RD->hasAttr<DLLImportAttr>() &&
+               ShouldUseExternalRTTIDescriptor(CGM, Ty)) {
+      DLLStorageClass = llvm::GlobalValue::DLLImportStorageClass;
----------------
Similarly, `ShouldUseExternalRTTIDescriptor(CGM, Ty)` should always return false here because of the if statement, so this code is dead.


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3408
 
   if (CGM.getTriple().isWindowsItaniumEnvironment()) {
+    TypeName->setDLLStorageClass(DLLStorageClass);
----------------
You can remove the if statement, setting the storage class to default has no effect here because it is initialized to default.


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3411-3417
+    if (DLLStorageClass == llvm::GlobalValue::DLLImportStorageClass) {
       // Because the typename and the typeinfo are DLL import, convert them to
       // declarations rather than definitions.  The initializers still need to
       // be constructed to calculate the type for the declarations.
       TypeName->setInitializer(nullptr);
       GV->setInitializer(nullptr);
     }
----------------
This code is also dead.


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3704
 
-void ItaniumCXXABI::EmitFundamentalRTTIDescriptors(bool DLLExport) {
+void ItaniumCXXABI::EmitFundamentalRTTIDescriptors(
+    const TypeInfoOptions* Options) {
----------------
You might consider inlining `EmitFundamentalRTTIDescriptor` into this function and having it take a `CXXRecordDecl`. Then you wouldn't need to pass the `TypeInfoOptions` structure around, you can just compute the properties in this function outside of the loop.


https://reviews.llvm.org/D49109