[libcxx-commits] [PATCH] D121231: [libc++] Remove raw call to debug handler from __char_traits_length_checked

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision as: libc++.
ldionne added a comment.

In D121231#3368770 <https://reviews.llvm.org/D121231#3368770>, @EricWF wrote:

> Please don't change this. Google uses this hook to change the behavior of string_view internally. We're working to get rid of the patch, but this helper function was specifically put in place to allow for the patch.

I said something along those lines on Discord, but I'll repeat it here. It is unreasonable for upstream to halt progress based on downstream hacks that are invisible to upstream (remember the `_VSTD::` change?). Libc++ is used by hundreds of organizations, we literally couldn't do much at all if we always waited for folks to remove their hacks before making a change. In practice, what you should do is simply revert this patch internally until you've managed to remove the workaround -- it's fairly customary for that to happen, at least here at Apple. I wouldn't push for this change if it were just a cleanup, however it is necessary as part of the larger effort to enable assertions and clean up the debug mode (see D121123 <https://reviews.llvm.org/D121123> & friends).

Also, it is quite possible that the `string_view` hook you have is something that could be more generally useful -- is there any reason not to simply upstream it, or upstream a form of it that makes your downstream diff easier to maintain? I'm not looking to create trouble for you if that can be avoided.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121231