[libcxx-commits] [PATCH] D101293: [libc++] Move <__sso_allocator> out of include/ into src/. NFCI.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone marked 2 inline comments as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/src/CMakeLists.txt:67
   list(APPEND LIBCXX_SOURCES
+    include/locale_sso_allocator.h
     ios.cpp
----------------
Mordante wrote:
> Quuxplusone wrote:
> > ldionne wrote:
> > > Why is that needed? We normally don't add headers to the list of sources?
> > I agree that AFAIK from my knowledge of CMake, it should not be needed. However, look at lines 19-21, 32-43, 89-90, 101-104.
> > I'd be happy to kill off those lines at the same time as this one (in fact I'll update this patch to do so), but if we keep those lines then I think we should keep this one too, because presumably they're all there for the same reason (or non-reason).
> > 
> > I see the comment `Add all the headers to the project for IDEs` on line 118. Is that the rationale for adding these .h files to `LIBCXX_SOURCES`?
> CMake indeed only needs for the IDE. (Unless the header is generated, but that's not the case here.)
@Mordante: "CMake indeed only needs for the IDE" — So, would you say we should keep all the files in CMakeLists.txt for the benefit of IDE-users? Your choice of the word "...only..." is throwing me a little bit. :)

I.e. is this part of the current patch (removing 20 lines from this file) a good idea or a bad idea?  (I'm conservatively leaning toward "bad idea".)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101293