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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
