[libcxx-commits] [PATCH] D105932: [libcxx][modules] protects users from relying on libc++ detail headers (1/n)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM but it would be great to see CI passing before committing this!



================
Comment at: libcxx/test/libcxx/diagnostics/detail.headers/algorithm/adjacent_find.module.verify.cpp:1
+
+// -*- C++ -*-
----------------
cjdb wrote:
> ldionne wrote:
> > All your generated headers have this newline at the beginning -- they shouldn't.
> > 
> > I also think they should have a newline at the end (might not be required technically but we always do that).
> > All your generated headers have this newline at the beginning -- they shouldn't.
> 
> Is that really a concern? (I'm going to fix it, but I wanna understand why.)
> 
> > I also think they should have a newline at the end (might not be required technically but we always do that).
> 
> They already do. Evidence: Phab will tell you "no newline before EOF".
It's not a concern per-se, but we might as well generate files the way we'd do it by hand.

I guess a minor concern is that you want the modline at the top to be the very first line (but I would have little love for an editor that can't figure out syntax highlighting from the `.cpp` extension).


================
Comment at: libcxx/utils/CMakeLists.txt:16
+    COMMAND "${Python3_EXECUTABLE}" "${LIBCXX_SOURCE_DIR}/utils/generate_private_header_tests.py"
+    COMMENT "Generates tests for ensuring detail headers are private")
+
----------------
When you commit, please change this `COMMENT` to the imperative, like the others above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105932