<div dir="ltr">On 13 October 2015 at 15:12, Eric Fiselier via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ericwf<br>
Date: Tue Oct 13 17:12:02 2015<br>
New Revision: 250235<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=250235&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=250235&view=rev</a><br>
Log:<br>
[libcxx] Capture configuration information when installing the libc++ headers<br>
<br>
Summary:<br>
Hi all,<br>
<br>
This patch is a successor to D11963. However it has changed dramatically and I felt it would be best to start a new review thread.<br>
<br>
Please read the design documentation added in this patch for a description of how it works.<br>
<br>
Reviewers: mclow.lists, danalbert, jroelofs, EricWF<br>
<br>
Subscribers: vkalintiris, rnk, ed, espositofulvio, asl, eugenis, cfe-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D13407" rel="noreferrer" target="_blank">http://reviews.llvm.org/D13407</a><br>
<br>
Added:<br>
    libcxx/trunk/docs/DesignDocs/<br>
    libcxx/trunk/docs/DesignDocs/<wbr>CapturingConfigInfo.rst<br>
    libcxx/trunk/include/__<a href="http://config_site.in" rel="noreferrer" target="_blank">config_<wbr>site.in</a><br>
Modified:<br>
    libcxx/trunk/CMakeLists.txt<br>
    libcxx/trunk/cmake/Modules/<wbr>HandleLibcxxFlags.cmake<br>
    libcxx/trunk/docs/index.rst<br>
    libcxx/trunk/include/<wbr>CMakeLists.txt<br>
<br>
Modified: libcxx/trunk/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.txt?rev=250235&r1=250234&r2=250235&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/libcxx/trunk/<wbr>CMakeLists.txt?rev=250235&r1=<wbr>250234&r2=250235&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- libcxx/trunk/CMakeLists.txt (original)<br>
+++ libcxx/trunk/CMakeLists.txt Tue Oct 13 17:12:02 2015<br>
@@ -260,13 +260,6 @@ endif()<br>
<br>
 # Feature flags ==============================<wbr>==============================<wbr>===<br>
 define_if(MSVC -D_CRT_SECURE_NO_WARNINGS)<br>
-define_if_not(LIBCXX_ENABLE_<wbr>GLOBAL_FILESYSTEM_NAMESPACE -D_LIBCPP_HAS_NO_GLOBAL_<wbr>FILESYSTEM_NAMESPACE)<br>
-define_if_not(LIBCXX_ENABLE_<wbr>STDIN -D_LIBCPP_HAS_NO_STDIN)<br>
-define_if_not(LIBCXX_ENABLE_<wbr>STDOUT -D_LIBCPP_HAS_NO_STDOUT)<br>
-define_if_not(LIBCXX_ENABLE_<wbr>THREADS -D_LIBCPP_HAS_NO_THREADS)<br>
-define_if_not(LIBCXX_ENABLE_<wbr>MONOTONIC_CLOCK -D_LIBCPP_HAS_NO_MONOTONIC_<wbr>CLOCK)<br>
-define_if_not(LIBCXX_ENABLE_<wbr>THREAD_UNSAFE_C_FUNCTIONS -D_LIBCPP_HAS_NO_THREAD_<wbr>UNSAFE_C_FUNCTIONS)<br>
-<br>
<br>
 # Sanitizer flags ==============================<wbr>==============================<wbr>=<br>
<br>
@@ -303,6 +296,22 @@ if (LIBCXX_BUILT_STANDALONE)<br>
     message(WARNING "LLVM_USE_SANITIZER is not supported on this platform.")<br>
   endif()<br>
 endif()<br>
+<br>
+# Configuration file flags ==============================<wbr>=======================<br>
+config_define_if_not(LIBCXX_<wbr>ENABLE_GLOBAL_FILESYSTEM_<wbr>NAMESPACE _LIBCPP_HAS_NO_GLOBAL_<wbr>FILESYSTEM_NAMESPACE)<br>
+config_define_if_not(LIBCXX_<wbr>ENABLE_STDIN _LIBCPP_HAS_NO_STDIN)<br>
+config_define_if_not(LIBCXX_<wbr>ENABLE_STDOUT _LIBCPP_HAS_NO_STDOUT)<br>
+config_define_if_not(LIBCXX_<wbr>ENABLE_THREADS _LIBCPP_HAS_NO_THREADS)<br>
+config_define_if_not(LIBCXX_<wbr>ENABLE_MONOTONIC_CLOCK _LIBCPP_HAS_NO_MONOTONIC_<wbr>CLOCK)<br>
+config_define_if_not(LIBCXX_<wbr>ENABLE_THREAD_UNSAFE_C_<wbr>FUNCTIONS _LIBCPP_HAS_NO_THREAD_UNSAFE_<wbr>C_FUNCTIONS)<br>
+<br>
+if (LIBCXX_NEEDS_SITE_CONFIG)<br>
+  configure_file(<br>
+    include/__<a href="http://config_site.in" rel="noreferrer" target="_blank">config_site.in</a><br>
+    ${LIBCXX_BINARY_DIR}/__config_<wbr>site<br>
+    @ONLY)<br>
+endif()<br>
+<br>
 #=============================<wbr>==============================<wbr>====================<br>
 # Setup Source Code And Tests<br>
 #=============================<wbr>==============================<wbr>====================<br>
<br>
Modified: libcxx/trunk/cmake/Modules/<wbr>HandleLibcxxFlags.cmake<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake?rev=250235&r1=250234&r2=250235&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/libcxx/trunk/cmake/<wbr>Modules/HandleLibcxxFlags.<wbr>cmake?rev=250235&r1=250234&r2=<wbr>250235&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- libcxx/trunk/cmake/Modules/<wbr>HandleLibcxxFlags.cmake (original)<br>
+++ libcxx/trunk/cmake/Modules/<wbr>HandleLibcxxFlags.cmake Tue Oct 13 17:12:02 2015<br>
@@ -49,6 +49,22 @@ macro(define_if_not condition def)<br>
   endif()<br>
 endmacro()<br>
<br>
+macro(config_define_if condition def)<br>
+  if (${condition})<br>
+    set(${def} ON)<br>
+    add_definitions(-D${def})<br>
+    set(LIBCXX_NEEDS_SITE_CONFIG ON)<br>
+  endif()<br>
+endmacro()<br>
+<br>
+macro(config_define_if_not condition def)<br>
+  if (NOT ${condition})<br>
+    set(${def} ON)<br>
+    add_definitions(-D${def})<br>
+    set(LIBCXX_NEEDS_SITE_CONFIG ON)<br>
+  endif()<br>
+endmacro()<br>
+<br>
 # Add a specified list of flags to both 'LIBCXX_COMPILE_FLAGS' and<br>
 # 'LIBCXX_LINK_FLAGS'.<br>
 macro(add_flags)<br>
<br>
Added: libcxx/trunk/docs/DesignDocs/<wbr>CapturingConfigInfo.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst?rev=250235&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/libcxx/trunk/docs/<wbr>DesignDocs/<wbr>CapturingConfigInfo.rst?rev=<wbr>250235&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- libcxx/trunk/docs/DesignDocs/<wbr>CapturingConfigInfo.rst (added)<br>
+++ libcxx/trunk/docs/DesignDocs/<wbr>CapturingConfigInfo.rst Tue Oct 13 17:12:02 2015<br>
@@ -0,0 +1,88 @@<br>
+=============================<wbr>==========================<br>
+Capturing configuration information during installation<br>
+=============================<wbr>==========================<br>
+<br>
+.. contents::<br>
+   :local:<br>
+<br>
+The Problem<br>
+===========<br>
+<br>
+Currently the libc++ supports building the library with a number of different<br>
+configuration options.  Unfortunately all of that configuration information is<br>
+lost when libc++ is installed. In order to support "persistent"<br>
+configurations libc++ needs a mechanism to capture the configuration options<br>
+in the INSTALLED headers.<br>
+<br>
+<br>
+Design Goals<br>
+============<br>
+<br>
+* The solution should not INSTALL any additional headers. We don't want an extra<br>
+  #include slowing everybody down.<br>
+<br>
+* The solution should not unduly affect libc++ developers. The problem is limited<br>
+  to installed versions of libc++ and the solution should be as well.<br>
+<br>
+* The solution should not modify any existing headers EXCEPT during installation.<br>
+  It makes developers lives harder if they have to regenerate the libc++ headers<br>
+  every time they are modified.<br>
+<br>
+* The solution should not make any of the libc++ headers dependant on<br>
+  files generated by the build system. The headers should be able to compile<br>
+  out of the box without any modification.<br>
+<br>
+* The solution should not have ANY effect on users who don't need special<br>
+  configuration options. The vast majority of users will never need this so it<br>
+  shouldn't cost them.<br>
+<br>
+<br>
+The Solution<br>
+============<br>
+<br>
+When you first configure libc++ using CMake we check to see if we need to<br>
+capture any options. If we haven't been given any "persistent" options then<br>
+we do NOTHING.<br>
+<br>
+Otherwise we create a custom installation rule that modifies the installed __config<br>
+header. The rule first generates a dummy "__config_site" header containing the required<br>
+#defines. The contents of the dummy header are then prependend to the installed<br>
+__config header. By manually prepending the files we avoid the cost of an<br>
+extra #include and we allow the __config header to be ignorant of the extra<br>
+ configuration all together. An example "__config" header generated when<br>
+-DLIBCXX_ENABLE_THREADS=OFF is given to CMake would look something like:<br>
+<br>
+.. code-block:: cpp<br>
+<br>
+  //===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
+  //<br>
+  //                     The LLVM Compiler Infrastructure<br>
+  //<br>
+  // This file is dual licensed under the MIT and the University of Illinois Open<br>
+  // Source Licenses. See LICENSE.TXT for details.<br>
+  //<br>
+  //===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
+<br>
+  #ifndef _LIBCPP_CONFIG_SITE<br>
+  #define _LIBCPP_CONFIG_SITE<br>
+<br>
+  /* #undef _LIBCPP_HAS_NO_GLOBAL_<wbr>FILESYSTEM_NAMESPACE */<br>
+  /* #undef _LIBCPP_HAS_NO_STDIN */<br>
+  /* #undef _LIBCPP_HAS_NO_STDOUT */<br>
+  #define _LIBCPP_HAS_NO_THREADS<br>
+  /* #undef _LIBCPP_HAS_NO_MONOTONIC_CLOCK */<br>
+  /* #undef _LIBCPP_HAS_NO_THREAD_UNSAFE_<wbr>C_FUNCTIONS */<br>
+<br>
+  #endif<br>
+  // -*- C++ -*-<br>
+  //===-------------------------<wbr>-- __config ------------------------------<wbr>---===//<br>
+  //<br>
+  //                     The LLVM Compiler Infrastructure<br>
+  //<br>
+  // This file is dual licensed under the MIT and the University of Illinois Open<br>
+  // Source Licenses. See LICENSE.TXT for details.<br>
+  //<br>
+  //===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
+<br>
+  #ifndef _LIBCPP_CONFIG<br>
+  #define _LIBCPP_CONFIG<br>
<br>
Modified: libcxx/trunk/docs/index.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/index.rst?rev=250235&r1=250234&r2=250235&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/libcxx/trunk/docs/<wbr>index.rst?rev=250235&r1=<wbr>250234&r2=250235&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- libcxx/trunk/docs/index.rst (original)<br>
+++ libcxx/trunk/docs/index.rst Tue Oct 13 17:12:02 2015<br>
@@ -124,6 +124,12 @@ A full list of currently open libc++ bug<br>
 Design Documents<br>
 ----------------<br>
<br>
+.. toctree::<br>
+   :maxdepth: 1<br>
+<br>
+   DesignDocs/CapturingConfigInfo<br>
+<br>
+<br>
 * `<atomic> design <<a href="http://libcxx.llvm.org/atomic_design.html" rel="noreferrer" target="_blank">http://libcxx.llvm.org/<wbr>atomic_design.html</a>>`_<br>
 * `<type_traits> design <<a href="http://libcxx.llvm.org/type_traits_design.html" rel="noreferrer" target="_blank">http://libcxx.llvm.org/type_<wbr>traits_design.html</a>>`_<br>
 * `Status of debug mode <<a href="http://libcxx.llvm.org/debug_mode.html" rel="noreferrer" target="_blank">http://libcxx.llvm.org/debug_<wbr>mode.html</a>>`_<br>
<br>
Modified: libcxx/trunk/include/<wbr>CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/CMakeLists.txt?rev=250235&r1=250234&r2=250235&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/libcxx/trunk/include/<wbr>CMakeLists.txt?rev=250235&r1=<wbr>250234&r2=250235&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- libcxx/trunk/include/<wbr>CMakeLists.txt (original)<br>
+++ libcxx/trunk/include/<wbr>CMakeLists.txt Tue Oct 13 17:12:02 2015<br>
@@ -1,10 +1,12 @@<br>
 if (NOT LIBCXX_INSTALL_SUPPORT_<wbr>HEADERS)<br>
   set(LIBCXX_SUPPORT_HEADER_<wbr>PATTERN PATTERN "support" EXCLUDE)<br>
 endif()<br>
+<br>
 set(LIBCXX_HEADER_PATTERN<br>
   PATTERN "*"<br>
   PATTERN "CMakeLists.txt" EXCLUDE<br>
   PATTERN ".svn" EXCLUDE<br>
+  PATTERN "__<a href="http://config_site.in" rel="noreferrer" target="_blank">config_site.in</a>" EXCLUDE<br>
   ${LIBCXX_SUPPORT_HEADER_<wbr>PATTERN}<br>
   )<br>
<br>
@@ -22,4 +24,29 @@ if (LIBCXX_INSTALL_HEADERS)<br>
     ${LIBCXX_HEADER_PATTERN}<br>
     PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ<br>
   )<br>
+<br>
+  if (LIBCXX_NEEDS_SITE_CONFIG)<br>
+    set(UNIX_CAT cat)<br>
+    if (WIN32)<br>
+      set(UNIX_CAT type)<br>
+    endif()<br>
+    # Generate and install a custom __config header. The new header is created<br>
+    # by  prepending __config_site to the current __config header.<br>
+    add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__<wbr>generated_config<br>
+      COMMAND ${CMAKE_COMMAND} -E copy ${LIBCXX_BINARY_DIR}/__config_<wbr>site ${LIBCXX_BINARY_DIR}/__<wbr>generated_config<br>
+      COMMAND ${UNIX_CAT} ${LIBCXX_SOURCE_DIR}/include/_<wbr>_config >> ${LIBCXX_BINARY_DIR}/__<wbr>generated_config<br>
+      DEPENDS ${LIBCXX_SOURCE_DIR}/include/_<wbr>_config<br>
+              ${LIBCXX_BINARY_DIR}/__config_<wbr>site<br>
+    )<br></blockquote><div><br></div><div>Hi Eric, sorry for the very late review comments!</div><div><br></div><div>This seems like a bad approach for dealing with configuration. Concatenating multiple files into a single header like this breaks include guard detection; for a file like __config, which is fairly long and intended to be included dozens of times into each compilation, that's particularly bad. It also means that sharing a libc++ installation across configurations requires duplicating the entire __config file, not just the parts that actually change between configurations.</div><div><br></div><div>As an alternative, how about this:</div><div><br></div><div>Provide a dummy __config_site in the svn include directory, and make __config unconditionally #include __config_site. On install, configure the __<a href="http://config_site.in">config_site.in</a> file to produce an installed __config_site rather than copying the dummy one.</div><div><br></div><div>Would that address your needs?</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    # Add a target that executes the generation commands.<br>
+    add_custom_target(generate_<wbr>config_header ALL<br>
+      DEPENDS ${LIBCXX_BINARY_DIR}/__<wbr>generated_config)<br>
+    # Install the generated header as __config.<br>
+    install(FILES ${LIBCXX_BINARY_DIR}/__<wbr>generated_config<br>
+      DESTINATION include/c++/v1<br>
+      PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ<br>
+      RENAME __config<br>
+      COMPONENT libcxx)<br>
+  endif()<br>
+<br>
 endif()<br>
<br>
Added: libcxx/trunk/include/__<a href="http://config_site.in" rel="noreferrer" target="_blank">config_<wbr>site.in</a><br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config_site.in?rev=250235&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/libcxx/trunk/include/_<wbr>_config_site.in?rev=250235&<wbr>view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- libcxx/trunk/include/__<a href="http://config_site.in" rel="noreferrer" target="_blank">config_<wbr>site.in</a> (added)<br>
+++ libcxx/trunk/include/__<a href="http://config_site.in" rel="noreferrer" target="_blank">config_<wbr>site.in</a> Tue Oct 13 17:12:02 2015<br>
@@ -0,0 +1,20 @@<br>
+//===------------------------<wbr>------------------------------<wbr>----------------===//<br>
+//<br>
+//                     The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is dual licensed under the MIT and the University of Illinois Open<br>
+// Source Licenses. See LICENSE.TXT for details.<br>
+//<br>
+//===------------------------<wbr>------------------------------<wbr>----------------===//<br>
+<br>
+#ifndef _LIBCPP_CONFIG_SITE<br>
+#define _LIBCPP_CONFIG_SITE<br>
+<br>
+#cmakedefine _LIBCPP_HAS_NO_GLOBAL_<wbr>FILESYSTEM_NAMESPACE<br>
+#cmakedefine _LIBCPP_HAS_NO_STDIN<br>
+#cmakedefine _LIBCPP_HAS_NO_STDOUT<br>
+#cmakedefine _LIBCPP_HAS_NO_THREADS<br>
+#cmakedefine _LIBCPP_HAS_NO_MONOTONIC_CLOCK<br>
+#cmakedefine _LIBCPP_HAS_NO_THREAD_UNSAFE_<wbr>C_FUNCTIONS<br>
+<br>
+#endif<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>