[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:51:00 PDT 2017
On 13 September 2017 at 12:15, Eric Fiselier via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> On Wed, Sep 13, 2017 at 1:02 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> 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/Modul
>>> es/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/Design
>>> Docs/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/CMa
>>> keLists.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.
>>
>
> Ouch, breaking the include guard detection really sucks. I'll make sure
> that gets fixed.
>
> I'm not too concerned about duplicating the `__config` file across
> configurations. For the most part __config_site options are ABI breaking,
> so changing any of the options typically means requiring a new dylib. Where
> do you envision this use case occurring?
>
What I had in mind was a multilib system that shares the include/ directory
(other than a few factored-out target-specific files) across all
architectures and configurations, and uses (say) different libc++ ABIs for
different architectures (and, naturally, different dylibs). That said,
having a different __config for each such target is really only a minor
problem. It's not like we want to support people using the same
__config_site for different libc++ versions, so the fact that it contains
libc++ source code in addition to config settings is a nuisance at worst.
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?
>>
>>
>
> That would. That solution was initially rejected to avoid the cost of
> adding an extra include (You know; the whole libc++ uses monolithic headers
> philosophy). I've never actually produced a decent test that demonstrates
> if this cost is real or imagined, but it is clear now that it can't be
> worse that breaking include guard detection. Thoughts?
>
> @Marshall does this sound good to you?
>
> This solution would be much simpler and cleaner in the end.
>
>
>
>
>> + # 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/__c
>>> onfig_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
>>>
>>
>>
>
> _______________________________________________
> 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/182b63fe/attachment-0001.html>
More information about the cfe-commits
mailing list