[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

This looks fine to me, but this definitely should have an accompanying test.

As to the other items you mention, the current section naming actually is important as it enables the CFString structures to be located (the linker will create `__start_cfstring` and `__stop_cfstring` synthetic symbols to mark the beginning and ending of the section), allowing the runtime to get to the section (the section is meant to be an array of objects).  Additionally, the section is not marked as constant because the runtime can actually change the contents (`CFConstantString` is *not* constant as you would think, the underlying value can be changed as can the `isa` pointer).



================
Comment at: lib/CodeGen/CodeGenModule.cpp:4101
     Ty = llvm::ArrayType::get(Ty, 0);
-    llvm::GlobalValue *GV = cast<llvm::GlobalValue>(
-        CreateRuntimeVariable(Ty, "__CFConstantStringClassReference"));
-
+    llvm::Constant *CGV =
+        CreateRuntimeVariable(Ty, "__CFConstantStringClassReference");
----------------
`CGV` doesn't really follow the expectation (`GV` was short for `GlobalValue`, this is a `Constant` so `C` seems more appropriate).


================
Comment at: lib/CodeGen/CodeGenModule.cpp:4108
+      // consistent with old behavior for this target as it would fail
+      // on the cast<> instead.
+      assert(GV && "isa<CFConstantStringClassReference> isn't a GlobalValue");
----------------
I think that the comment isn't particularly helpful - it basically is directing you to look at the history of the file.


Repository:
  rC Clang

https://reviews.llvm.org/D52344