[llvm-branch-commits] [clang] [flang] [lld] [llvm] [Flang] LLVM_ENABLE_RUNTIMES=flang-rt (PR #110217)

[Joseph Huber via llvm-branch-commits]

jhuber6 wrote:

> > Overall I think the patch is fine pending some naming nits, my one concern is the `-U__GLIBCXX` stuff, because undefining internal vars seems really sketchy. Do we use `-nostdlib++` to make sure we don't link the C++ library?
> 
> There is a [test](https://github.com/llvm/llvm-project/blob/main/flang/test/Runtime/no-cpp-dep.c) that uses a C-compiler to link the runtime. This more portable than `-nostdlib++`.
> 
> [`_GLIBCXX_ASSERTIONS`](https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html) and [`_LIBCPP_ENABLE_ASSERTIONS`](https://releases.llvm.org/17.0.1/projects/libcxx/docs/UsingLibcxx.html) are used as described by their respective libraries and how it is done before this PR. For libstdc++ defining 

We should do `check_cxx_compiler_flag(-nostdlib++ FLANG_RT_HAS_NOSTDLIBPP)`, it's a lot more obvious when your program doesn't link than when a test fails (but the test is still good).

> `_GLIBCXX_NO_ASSERTIONS=1` might be better than undefining something. For libc++ `_LIBCPP_ENABLE_ASSERTIONS` has been [deprecated](https://reviews.llvm.org/D154997) in favor of [`_LIBCPP_HARDENING_MODE`](https://libcxx.llvm.org/Hardening.html). Changing that would be a different concern than addressed by this PR.

Yeah it's just copying what's already there so it's not a blocker. I agree that defining them explicitly is way better, because then if someone redefines them you'll get a warning.

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