[PATCH] D141446: Fix to D139603(reverted) - moved size check to unit test so that it is cross-platform

Wed Jan 11 20:27:27 PST 2023

thakis added a comment.

This unit-test based test also fails on non-linux, see e.g. http://45.33.8.238/macm1/52570/step_11.txt

I think that's because `writeWithSizeLimitInternal` calls `Strategy->Erase(StringBuffer.size());` and that erases an element from an `unordered_map`, and it depends on the C++ standard library if an "important" function name gets removed. Depending on that, the error is either `too_large` (which the test expects) or `truncated_name_table` (which it doesn't).

Please take a look and revert for now if it takes a while to fix. If you do end up reverting, it'd be cool if you could revert 6be251352e6b4d9708a1b7b7b146ea199342de22 <https://reviews.llvm.org/rG6be251352e6b4d9708a1b7b7b146ea199342de22> in the same commit (and reland it when you reland).

================
Comment at: llvm/unittests/tools/llvm-profdata/OutputSizeLimitTest.cpp:37
+static void CheckAssertFalse(std::error_code t) {
+  ASSERT_FALSE(t) << t.message().c_str();
+}
----------------
https://github.com/google/googletest/blob/main/docs/advanced.md#propagating-fatal-failures FYI

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141446