theraven added inline comments.
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:977
+ if ((CGM.getTarget().getPointerWidth(0) == 64) &&
+ (SL->getLength() < 9) && !isNonASCII) {
+ // Tiny strings are (roughly):
----------------
rjmccall wrote:
> Please hoist `SL->getLength()` into a variable.
>
> Also, consider breaking this bit of code (maybe just building a `uint64_t`?)
> into a separate function.
I don't think separating it would simplify the code much. It's 8 lines of
non-comment code and you'd probably need at least three of them in the caller.
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:1077
+ std::replace(StringName.begin(), StringName.end(),
+ '@', '\1');
+ auto *ObjCStrGV =
----------------
rjmccall wrote:
> Is `@` really the only illegal character in ELF symbol names? Surely at
> least `\0` as well. And your mangling will collide strings if they contain
> `\1`.
>
> I think you should probably just have this use internal linkage (and a
> meaningless symbol name) for strings containing bad characters.
No, I copied the mangling for selectors without engaging my brain. I've now
replaced it with something much more conservative. I might tweak it a bit
before the next release, but this seems to give pretty good coverage.
Repository:
rC Clang
https://reviews.llvm.org/D46052
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
