[PATCH] D46665: [Itanium] Emit type info names with external linkage.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked an inline comment as done.
EricWF added a comment.

@rsmith What should the visibility of the type name be in cases where the type info is hidden?



================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3033
+      return {llvm::GlobalValue::InternalLinkage,
+              llvm::GlobalValue::LinkOnceODRLinkage};
+    return {llvm::GlobalValue::InternalLinkage,
----------------
rsmith wrote:
> Shouldn't this be based on the type's linkage? Eg:
> 
> ```
> namespace { struct Incomplete; }
> auto x = typeid(Incomplete*);
> ```
> 
> should have an `internal` type info name, not a `linkonce_odr` name. I think we should compute the linkage per the code below and then demote the type's linkage to internal in the incomplete-class case.
That sounds correct to me.


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3034-3035
+              llvm::GlobalValue::LinkOnceODRLinkage};
+    return {llvm::GlobalValue::InternalLinkage,
+            llvm::GlobalValue::InternalLinkage};
+  }
----------------
rsmith wrote:
> Should we promote the type info name in this case too? (We get here for the incomplete-class type info we generate as a detail of the pointer type info, but the names of such types are accessible via the <cxx_abi.h> interface, so it seems reasonable to apply the same global-uniqueness to them too.)
Hmm. I think it seems reasonable.

I don't think there exists a case where a user can perform `type_info` equality or hashing on an incomplete class type,  so in terms of solving the libc++ bug I think it's unneeded. (which is why I omitted it).

I also don't see exactly what part of `cxxabi.h` might expose the `type_info` to users in a way where type name pointer equality is important to the user.

What would you rather?



https://reviews.llvm.org/D46665