[libcxx-commits] [libcxx] [libcxx] Remove Redundant Reset in ~basic_string (PR #164718)

[Aiden Grossman via libcxx-commits]

boomanaiden154 wrote:

> That is true, but I think you're misinterpreting. My understanding (which could be wrong as well) is that code gets inlined into the current function. Then, on that function, the following passes are run. After all the following passes are run, another function may inline the just optimized function.

Ah, I think this is where we're getting misaligned. I thought that DSE ran outside of the CGSCC pipeline, but it is actually inside the CGSCC pipeline, so adding `dead_on_return` actually should work. I guess I should actually look closely at the parentheses in the pass pipelines that I post. Sorry for the misunderstanding.

So I think we can do the following:
1. Land this patch with a TODO. We can either land as-is, or with Louis' suggested refactoring. Let me know which you prefer.
2. I will write up a patch to make `clang` mark the this pointer `dead_on_return` inside base class destructors. Probably gated under an off by default flag initially.
3. Evaluate impact on our internal code base (both performance and see how many people are relying on the UB being a certain way).
4. Maybe enable by default.

I don't want to give any timelines/promises on number four because if the fallout turns out to be large like Reid has predicted, it might be hard for us to find resourcing to clean everything up (although it would be good either way).

https://github.com/llvm/llvm-project/pull/164718