[PATCH] D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal.

Thu May 2 15:24:12 PDT 2019

ahatanak added inline comments.

================
Comment at: lib/CodeGen/CGObjCMac.cpp:1978
+                                        ? llvm::GlobalVariable::InternalLinkage
+                                        : llvm::GlobalValue::PrivateLinkage);
   // FIXME. Fix section.
----------------
rjmccall wrote:
> 1. Why are these using different classes to name these enumerators?
> 2. Why are we promoting to internal linkage in some cases?  That seems to be an additional semantic change not discussed in the commit message, and it should probably also get a good inline comment.
This is a mistake. It should have been `llvm::GlobalValue::InternalLinkage`.

================
Comment at: lib/CodeGen/CGObjCMac.cpp:1983
+  else
+    GV->setSection("__OBJC,__cstring_object,regular,no_dead_strip");
+
----------------
vsk wrote:
> Are constant NSStrings within the shared cache ever dirtied? I was under the impression that they couldn't be, so I'm not sure there's an advantage to making them internal.
I was making all symbols in `__DATA` internal, but you are right, there isn't any advantage to making constant NSStrings internal for the purpose of this patch.

Repository:
  rC Clang

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

https://reviews.llvm.org/D61454