On 13 October 2015 at 15:12, Eric Fiselier via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> Author: ericwf > Date: Tue Oct 13 17:12:02 2015 > New Revision: 250235 > > URL: http://llvm.org/viewvc/llvm-project?rev=250235&view=rev > Log: > [libcxx] Capture configuration information when installing the libc++ > headers > > Summary: > Hi all, > > 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. > > Please read the design documentation added in this patch for a description > of how it works. > > Reviewers: mclow.lists, danalbert, jroelofs, EricWF > > Subscribers: vkalintiris, rnk, ed, espositofulvio, asl, eugenis, > cfe-commits > > Differential Revision: http://reviews.llvm.org/D13407 > > Added: > libcxx/trunk/docs/DesignDocs/ > libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst > libcxx/trunk/include/__config_site.in > Modified: > libcxx/trunk/CMakeLists.txt > libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake > libcxx/trunk/docs/index.rst > libcxx/trunk/include/CMakeLists.txt > > Modified: libcxx/trunk/CMakeLists.txt > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/ > CMakeLists.txt?rev=250235&r1=250234&r2=250235&view=diff > ============================================================ > ================== > --- libcxx/trunk/CMakeLists.txt (original) > +++ libcxx/trunk/CMakeLists.txt Tue Oct 13 17:12:02 2015 > @@ -260,13 +260,6 @@ endif() > > # Feature flags ============================== > ================================= > define_if(MSVC -D_CRT_SECURE_NO_WARNINGS) > -define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE > -D_LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE) > -define_if_not(LIBCXX_ENABLE_STDIN -D_LIBCPP_HAS_NO_STDIN) > -define_if_not(LIBCXX_ENABLE_STDOUT -D_LIBCPP_HAS_NO_STDOUT) > -define_if_not(LIBCXX_ENABLE_THREADS -D_LIBCPP_HAS_NO_THREADS) > -define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK -D_LIBCPP_HAS_NO_MONOTONIC_ > CLOCK) > -define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS > -D_LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS) > - > > # Sanitizer flags ============================== > =============================== > > @@ -303,6 +296,22 @@ if (LIBCXX_BUILT_STANDALONE) > message(WARNING "LLVM_USE_SANITIZER is not supported on this > platform.") > endif() > endif() > + > +# Configuration file flags ============================== > ======================= > +config_define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE > _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE) > +config_define_if_not(LIBCXX_ENABLE_STDIN _LIBCPP_HAS_NO_STDIN) > +config_define_if_not(LIBCXX_ENABLE_STDOUT _LIBCPP_HAS_NO_STDOUT) > +config_define_if_not(LIBCXX_ENABLE_THREADS _LIBCPP_HAS_NO_THREADS) > +config_define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK > _LIBCPP_HAS_NO_MONOTONIC_CLOCK) > +config_define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS > _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS) > + > +if (LIBCXX_NEEDS_SITE_CONFIG) > + configure_file( > + include/__config_site.in > + ${LIBCXX_BINARY_DIR}/__config_site > + @ONLY) > +endif() > + > #=========================================================== > ==================== > # Setup Source Code And Tests > #=========================================================== > ==================== > > Modified: libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/ > Modules/HandleLibcxxFlags.cmake?rev=250235&r1=250234&r2=250235&view=diff > ============================================================ > ================== > --- libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake (original) > +++ libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake Tue Oct 13 > 17:12:02 2015 > @@ -49,6 +49,22 @@ macro(define_if_not condition def) > endif() > endmacro() > > +macro(config_define_if condition def) > + if (${condition}) > + set(${def} ON) > + add_definitions(-D${def}) > + set(LIBCXX_NEEDS_SITE_CONFIG ON) > + endif() > +endmacro() > + > +macro(config_define_if_not condition def) > + if (NOT ${condition}) > + set(${def} ON) > + add_definitions(-D${def}) > + set(LIBCXX_NEEDS_SITE_CONFIG ON) > + endif() > +endmacro() > + > # Add a specified list of flags to both 'LIBCXX_COMPILE_FLAGS' and > # 'LIBCXX_LINK_FLAGS'. > macro(add_flags) > > Added: libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/ > CapturingConfigInfo.rst?rev=250235&view=auto > ============================================================ > ================== > --- libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst (added) > +++ libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst Tue Oct 13 > 17:12:02 2015 > @@ -0,0 +1,88 @@ > +======================================================= > +Capturing configuration information during installation > +======================================================= > + > +.. contents:: > + :local: > + > +The Problem > +=========== > + > +Currently the libc++ supports building the library with a number of > different > +configuration options. Unfortunately all of that configuration > information is > +lost when libc++ is installed. In order to support "persistent" > +configurations libc++ needs a mechanism to capture the configuration > options > +in the INSTALLED headers. > + > + > +Design Goals > +============ > + > +* The solution should not INSTALL any additional headers. We don't want > an extra > + #include slowing everybody down. > + > +* The solution should not unduly affect libc++ developers. The problem is > limited > + to installed versions of libc++ and the solution should be as well. > + > +* The solution should not modify any existing headers EXCEPT during > installation. > + It makes developers lives harder if they have to regenerate the libc++ > headers > + every time they are modified. > + > +* The solution should not make any of the libc++ headers dependant on > + files generated by the build system. The headers should be able to > compile > + out of the box without any modification. > + > +* The solution should not have ANY effect on users who don't need special > + configuration options. The vast majority of users will never need this > so it > + shouldn't cost them. > + > + > +The Solution > +============ > + > +When you first configure libc++ using CMake we check to see if we need to > +capture any options. If we haven't been given any "persistent" options > then > +we do NOTHING. > + > +Otherwise we create a custom installation rule that modifies the > installed __config > +header. The rule first generates a dummy "__config_site" header > containing the required > +#defines. The contents of the dummy header are then prependend to the > installed > +__config header. By manually prepending the files we avoid the cost of an > +extra #include and we allow the __config header to be ignorant of the > extra > + configuration all together. An example "__config" header generated when > +-DLIBCXX_ENABLE_THREADS=OFF is given to CMake would look something like: > + > +.. code-block:: cpp > + > + //===------------------------------------------------------- > ---------------===// > + // > + // The LLVM Compiler Infrastructure > + // > + // This file is dual licensed under the MIT and the University of > Illinois Open > + // Source Licenses. See LICENSE.TXT for details. > + // > + //===------------------------------------------------------- > ---------------===// > + > + #ifndef _LIBCPP_CONFIG_SITE > + #define _LIBCPP_CONFIG_SITE > + > + /* #undef _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE */ > + /* #undef _LIBCPP_HAS_NO_STDIN */ > + /* #undef _LIBCPP_HAS_NO_STDOUT */ > + #define _LIBCPP_HAS_NO_THREADS > + /* #undef _LIBCPP_HAS_NO_MONOTONIC_CLOCK */ > + /* #undef _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS */ > + > + #endif > + // -*- C++ -*- > + //===--------------------------- __config > ---------------------------------===// > + // > + // The LLVM Compiler Infrastructure > + // > + // This file is dual licensed under the MIT and the University of > Illinois Open > + // Source Licenses. See LICENSE.TXT for details. > + // > + //===------------------------------------------------------- > ---------------===// > + > + #ifndef _LIBCPP_CONFIG > + #define _LIBCPP_CONFIG > > Modified: libcxx/trunk/docs/index.rst > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/ > index.rst?rev=250235&r1=250234&r2=250235&view=diff > ============================================================ > ================== > --- libcxx/trunk/docs/index.rst (original) > +++ libcxx/trunk/docs/index.rst Tue Oct 13 17:12:02 2015 > @@ -124,6 +124,12 @@ A full list of currently open libc++ bug > Design Documents > ---------------- > > +.. toctree:: > + :maxdepth: 1 > + > + DesignDocs/CapturingConfigInfo > + > + > * `<atomic> design <http://libcxx.llvm.org/atomic_design.html>`_ > * `<type_traits> design <http://libcxx.llvm.org/type_traits_design.html > >`_ > * `Status of debug mode <http://libcxx.llvm.org/debug_mode.html>`_ > > Modified: libcxx/trunk/include/CMakeLists.txt > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/ > CMakeLists.txt?rev=250235&r1=250234&r2=250235&view=diff > ============================================================ > ================== > --- libcxx/trunk/include/CMakeLists.txt (original) > +++ libcxx/trunk/include/CMakeLists.txt Tue Oct 13 17:12:02 2015 > @@ -1,10 +1,12 @@ > if (NOT LIBCXX_INSTALL_SUPPORT_HEADERS) > set(LIBCXX_SUPPORT_HEADER_PATTERN PATTERN "support" EXCLUDE) > endif() > + > set(LIBCXX_HEADER_PATTERN > PATTERN "*" > PATTERN "CMakeLists.txt" EXCLUDE > PATTERN ".svn" EXCLUDE > + PATTERN "__config_site.in" EXCLUDE > ${LIBCXX_SUPPORT_HEADER_PATTERN} > ) > > @@ -22,4 +24,29 @@ if (LIBCXX_INSTALL_HEADERS) > ${LIBCXX_HEADER_PATTERN} > PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ > ) > + > + if (LIBCXX_NEEDS_SITE_CONFIG) > + set(UNIX_CAT cat) > + if (WIN32) > + set(UNIX_CAT type) > + endif() > + # Generate and install a custom __config header. The new header is > created > + # by prepending __config_site to the current __config header. > + add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config > + COMMAND ${CMAKE_COMMAND} -E copy ${LIBCXX_BINARY_DIR}/__config_site > ${LIBCXX_BINARY_DIR}/__generated_config > + COMMAND ${UNIX_CAT} ${LIBCXX_SOURCE_DIR}/include/__config >> > ${LIBCXX_BINARY_DIR}/__generated_config > + DEPENDS ${LIBCXX_SOURCE_DIR}/include/__config > + ${LIBCXX_BINARY_DIR}/__config_site > + ) > Hi Eric, sorry for the very late review comments! 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. As an alternative, how about this: Provide a dummy __config_site in the svn include directory, and make __config unconditionally #include __config_site. On install, configure the __config_site.in file to produce an installed __config_site rather than copying the dummy one. Would that address your needs? > + # Add a target that executes the generation commands. > + add_custom_target(generate_config_header ALL > + DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config) > + # Install the generated header as __config. > + install(FILES ${LIBCXX_BINARY_DIR}/__generated_config > + DESTINATION include/c++/v1 > + PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ > + RENAME __config > + COMPONENT libcxx) > + endif() > + > endif() > > Added: libcxx/trunk/include/__config_site.in > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/_ > _config_site.in?rev=250235&view=auto > ============================================================ > ================== > --- libcxx/trunk/include/__config_site.in (added) > +++ libcxx/trunk/include/__config_site.in Tue Oct 13 17:12:02 2015 > @@ -0,0 +1,20 @@ > +//===------------------------------------------------------ > ----------------===// > +// > +// The LLVM Compiler Infrastructure > +// > +// This file is dual licensed under the MIT and the University of > Illinois Open > +// Source Licenses. See LICENSE.TXT for details. > +// > +//===------------------------------------------------------ > ----------------===// > + > +#ifndef _LIBCPP_CONFIG_SITE > +#define _LIBCPP_CONFIG_SITE > + > +#cmakedefine _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE > +#cmakedefine _LIBCPP_HAS_NO_STDIN > +#cmakedefine _LIBCPP_HAS_NO_STDOUT > +#cmakedefine _LIBCPP_HAS_NO_THREADS > +#cmakedefine _LIBCPP_HAS_NO_MONOTONIC_CLOCK > +#cmakedefine _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS > + > +#endif > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
