[libcxx] r250235 - [libcxx] Capture configuration information when installing the libc++ headers
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 13 12:02:26 PDT 2017
On 13 October 2015 at 15:12, Eric Fiselier via cfe-commits <
cfe-commits at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170913/80b81fa4/attachment-0001.html>
More information about the cfe-commits
mailing list