[libcxx-commits] [PATCH] D99178: [libcxx] Disable c++experimental by default in DLL builds

Thu Apr 1 14:14:00 PDT 2021

mstorsjo added inline comments.

================
Comment at: libcxx/docs/BuildingLibcxx.rst:93
           -DLIBCXX_ENABLE_STATIC=NO               ^
-          -DLIBCXX_ENABLE_EXPERIMENTAL_LIBRARY=NO ^
           \path\to\libcxx
----------------
Mordante wrote:
> mstorsjo wrote:
> > Mordante wrote:
> > > I assume things go wrong if the user uses `-DLIBCXX_ENABLE_EXPERIMENTAL_LIBRARY=YES`. Can you document this isn't supported? I think it would be nice if CMake doesn't allow this invalid combination.
> > I kind of wanted to leave it possible to do that still, while you'd essentially be on your own in that case. But I guess I could at least add a cmake warning for that combination - or a hard error if others also feel that way.
> I'm mainly concerned with the "removing" of documentation. Before the documentation mentioned you shouldn't do `-DLIBCXX_ENABLE_EXPERIMENTAL_LIBRARY=YES`. After the patch is says nothing and using this will cause errors.  So I think it's a good thing CMake does the right thing by default, but I dislike the removal of clarity.
> 
> I think we should add some documentation here the combination isn't supported.
> 
> In my personal opinion I think we should add a hard error in CMake, something like:
> ```
> if (WIN32 AND LIBCXX_ENABLE_SHARED AND LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY)
>     message(fatal_error "The libc++experimental isn't available for Windows shared libraries")
> endif()
> ```
> But if you rather show a warning I won't object.
I'd rather keep such details out of this part of the docs - there's still some amount of ifs and buts around this configuration, so it's better if we can reduce some amount of the noise in it at least, But I guess I can make it an error on the cmake level - that would at least make the situation clearer.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99178