[libcxx-commits] [PATCH] D59999: Allow the compiler to optimize `string == "literal string"`.

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added a comment.

@EricWF asked me:

> Can you restate your concern over bypassing char_traits?



1. The //real// problem here is that the string code is externally instantiated in the dylib, and thus not available to be inlined. This patch in no way addresses that, merely works around it in this one particular case for one particular operation (equality comparison) on one particular kind of data (narrow character literals).  I'm not really fond of solutions like that.
2. There's been no attempt to show that this is a performance bottleneck; just assertions that "the status quo bloats and slows down callers for very common simple comparisons with literal strings". Improving a micro-benchmark may or may not have an effect on real-world code.
3. The standard is quite clear here. The comparison calls `string::compare`, which calls `char_traits::compare`. With this patch, we're no longer doing that. This is probably allowed by the "as if" rule, but still seems "wrong" to me.

People who do not care about ABI stability could (should?) remove the external template instantiations, and get the benefits of inlining here (and in other calls).  If someone were to do this, and report back, we would have data to argue with about whether or not this patch would make any difference to actual users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59999