[libcxx-commits] [PATCH] D103947: [libcxx] Fix using the vcruntime ABI with _HAS_EXCEPTIONS=0 defined

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/configs/llvm-libc++-shared-no-exception-clangcl.cfg.in:1
+# This testing configuration handles running the test suite against LLVM's libc++
+# using a DLL, with Clang-cl on Windows.
----------------
ldionne wrote:
> Why is this not handled by passing `-fno-exceptions` on the command-line when running the test suite? In other words, why don't we use the existing `shared-clangcl.cfg.in` configuration and add `enable_exceptions=False`?
I think the main point is that this isn’t a configuration that someone intentionally chooses, as a project wide configuration. For various historic reasons, some projects seem to be building smaller subsets of their code (e.g. a subset of their source files) with this define (`_HAS_EXCEPTIONS=0`) set. They don’t build with `-fno-exceptions` (or the MSVC/clang-cl equivalents). I.e. the end user code is built with exception handling codegen enabled, but vcruntime’s classes hidden.

(Also, building with `-fno-exceptions` doesn’t define `_HAS_EXCEPTIONS=0`. They’re totally independent toggles.)

If building with `LIBCXX_ENABLE_EXCEPTIONS=OFF`, you’d have the libc++ built with vcruntime exception classes hidden - but that’s not the situation that end users use. Testing that way would probably cover many aspects of the code, but it wouldn’t test what end users actually end up using.

End users have a libc++ built entirely normally, but some fraction of end users code is built with vcruntime exception classes hidden.

(The fact that the library is built differently than end user code is the same situation as `-fexception` vs `-fno-exceptions` on unix. The library will be built with exceptions enabled, while some end user object files may be built with `-fno-exceptions`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103947