[libcxx-commits] [PATCH] D108389: [libc++] Bypass calling exception-throwing functions in the dylib with -fno-exceptions

[Jordan Rupprecht via Phabricator via libcxx-commits]

rupprecht added a comment.

In D108389#3015689 <https://reviews.llvm.org/D108389#3015689>, @ldionne wrote:

> In D108389#2973427 <https://reviews.llvm.org/D108389#2973427>, @jyknight wrote:
>
>> (Additionally, its errors are more useful, too, e.g. showing the actual out of range index, which makes that all the more valuable to emit in both modes).
>
> 100% agree here, our errors suck. I think there's a bug report to improve that somewhere.
>
> In D108389#2973423 <https://reviews.llvm.org/D108389#2973423>, @rupprecht wrote:
>
>> [We experienced some churn from this commit, but so far just seems like brittle tests]
>
> Please let me know if you experience anything serious from this change. I would expect it's NFC for 99.9% of people, except if you were doing something shady. If that's not correct, I might learn something useful so please let me know.

We did a bit of cleanup internally. "shady" is a close enough word for all the instances we had. I think we have now resolved all issues related to this commit.

The most common (but very uninteresting) cleanup we did was death tests asserting an exception being thrown, e.g. call `at(something_out_of_range)` and assert the subprocess dies with "out_of_range" in the stack trace. (This is for build modes that use libc++ built with -fexceptions, but the rest of the TUs are built with -fno-exceptions).

One interesting cleanup was a test failure like this (pseudocode-ish):

  auto lambda = [&]() {
    std::vector<int> foo;
    foo.resize(100);
  };
  auto memory_used = InvokeAndMeasureMemory(lambda);
  EXPECT_EQ(memory_used, 800);

This started failing because `memory_used` is now 0 after this commit. IIUC, having more declarations available in the header gave the optimizer more information that led to completely getting rid of the `std::vector` usage, so it's an empty lambda.

We had another one that's way too complicated to explain here, but it was another case of "more information = better optimization = test failure because a test asserts fragile things". In general I wouldn't expect this to make a very performance impact, but it's on the positive side of zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108389