[PATCH] D68075: Do not #error if no OS is #defined

David Zarzycki via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 4 12:32:46 PDT 2019


davezarzycki added a comment.

In D68075#1695117 <https://reviews.llvm.org/D68075#1695117>, @ldionne wrote:

> In D68075#1695110 <https://reviews.llvm.org/D68075#1695110>, @ldionne wrote:
>
> > I kinda like the suggestion. I'm pretty confident we don't support compilers that don't implement `__has_include` anyway (maybe we technically do, but I'm sure nothing good happens in that case).
>
>
> However, even your suggestion doesn't answer the question of what's the purpose of setting `-DLIBCXX_ENABLE_THREADS=OFF` if we detect it automatically in the headers anyway. If we go down your route, I'd drop `-DLIBCXX_ENABLE_THREADS` completely from CMake (but that suggests a broader direction from the project where the headers should be usable without any configuration option).


An old boss of mine liked to say that "simple things should be simple and hard things should be possible". Working out of the box for most configurations most of the time is the simple case. If somebody wants to explicitly disable threading, they should be able to do that, but we shouldn't force most people to enable threading if it can be automatically detected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68075





More information about the llvm-commits mailing list