[libc++] Do not share an object library to create the static/shared libraries

[ldionne]

Summary

The problem with using an object library for doing this is that it prevents
the shared library and the static library from being built with the right
default flags. For example, CMake will build shared libraries with -fPIC
by default, but not static libraries. Using an object library to create
the shared library will prevent the right default flags for shared
libraries from being used.

As a side effect, this patch also localizes the logic related to building
a hermetic static library to the static library case, making clear that
this has no effect on the shared library.

[phosek]

LGTM, this looks great, I think we should do the same for libc++abi and libunwind. I've added @smeenai since he has CMake expertise.

libcxx/lib/CMakeLists.txt

209 ↗

(On Diff #190243)

CMake documentation says that [COMPILE_FLAGS is deprecated](https://cmake.org/cmake/help/v3.4/prop_tgt/COMPILE_FLAGS.html) and that we should be using COMPILE_OPTIONS or target_compile_options.

237 ↗

(On Diff #190243)

ditto

[ldionne]

libcxx/lib/CMakeLists.txt

209 ↗

(On Diff #190243)

If that's okay, I'd like to do that as a follow-up change. I'd do:

target_compile_options(cxx_shared PRIVATE ${LIBCXX_COMPILE_FLAGS})
target_link_options(cxx_shared PRIVATE ${LIBCXX_LINK_FLAGS})

The reason is that if this somehow breaks something (I'm worried that target_compile_options is additive where setting the property might overwrite the compile flags for that target, which could be different), at least I won't have to revert the whole patch.

[smeenai]

libcxx/lib/CMakeLists.txt

169 ↗

(On Diff #190243)

I think this can trivially be a function instead of a macro, which avoids potential future variable pollution.

209 ↗

(On Diff #190243)

One difference to be aware of (and this is really annoying behavior on CMake's part IMO) is that COMPILE_OPTIONS are deduplicated, such that if you e.g. tried setting COMPILE_OPTIONS to -isystem foo -isystem bar, CMake would transform into -isystem foo bar, which of course breaks horribly. Hopefully you won't run into that, but it's something to be aware of.

[smeenai]

The downside, of course, is that we end up compiling the same files twice for the static and shared libraries, but it makes sense if they're gonna have different flags.

[ldionne]

Transform macro into function.

[ldionne]

In D59248#1426883, @smeenai wrote:

The downside, of course, is that we end up compiling the same files twice for the static and shared libraries, but it makes sense if they're gonna have different flags.

Yes, they do have different flags. But you're right, we're doing a bit more work now. Fortunately, building libc++ is very fast.

libcxx/lib/CMakeLists.txt

169 ↗

(On Diff #190243)

Good point.

209 ↗

(On Diff #190243)

I didn't know that! It's good to know.

lib/CMakeLists.txt

add_link_flags(
     "-Wl,-force_symbols_not_weak_list,\"${CMAKE_CURRENT_SOURCE_DIR}/notweak.exp\""
     "-Wl,-force_symbols_weak_list,\"${CMAKE_CURRENT_SOURCE_DIR}/weak.exp\"")
 
   if (NOT LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
     add_link_flags("-Wl,-reexported_symbols_list,\"${CMAKE_CURRENT_SOURCE_DIR}/libc++abi-new-delete.exp\"")
   endif()
 endif()
 
-split_list(LIBCXX_COMPILE_FLAGS)
-split_list(LIBCXX_LINK_FLAGS)
-
-macro(cxx_object_library name)
-  cmake_parse_arguments(ARGS "" "" "DEFINES;FLAGS" ${ARGN})
-
-  # Add an object library that contains the compiled source files.
-  add_library(${name} OBJECT ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
+function(cxx_set_common_defines name)
   if(LIBCXX_CXX_ABI_HEADER_TARGET)
     add_dependencies(${name} ${LIBCXX_CXX_ABI_HEADER_TARGET})
   endif()
   if(WIN32 AND NOT MINGW)
     target_compile_definitions(${name}
                                PRIVATE
                                  # Ignore the -MSC_VER mismatch, as we may build
                                  # with a different compatibility version.
target_compile_definitions(${name}
                                  # Don't warn on the use of "deprecated"
                                  # "insecure" functions which are standards
                                  # specified.
                                  _CRT_SECURE_NO_WARNINGS
                                  # Use the ISO conforming behaviour for conversion
                                  # in printf, scanf.
                                  _CRT_STDIO_ISO_WIDE_SPECIFIERS)
   endif()
+endfunction()
 
-  if(ARGS_DEFINES)
-    target_compile_definitions(${name} PRIVATE ${ARGS_DEFINES})
-  endif()
-
-  set_target_properties(${name}
-    PROPERTIES
-      COMPILE_FLAGS ${LIBCXX_COMPILE_FLAGS}
-  )
-
-  if(ARGS_FLAGS)
-    target_compile_options(${name} PRIVATE ${ARGS_FLAGS})
-  endif()
-endmacro()
-
-if(LIBCXX_HERMETIC_STATIC_LIBRARY)
-  append_flags_if_supported(CXX_STATIC_OBJECTS_FLAGS -fvisibility=hidden)
-  append_flags_if_supported(CXX_STATIC_OBJECTS_FLAGS -fvisibility-global-new-delete-hidden)
-  cxx_object_library(cxx_static_objects
-    DEFINES _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS
-    FLAGS ${CXX_STATIC_OBJECTS_FLAGS})
-  cxx_object_library(cxx_shared_objects)
-  set(cxx_static_sources $<TARGET_OBJECTS:cxx_static_objects>)
-  set(cxx_shared_sources $<TARGET_OBJECTS:cxx_shared_objects>)
-else()
-  cxx_object_library(cxx_objects)
-  set(cxx_static_sources $<TARGET_OBJECTS:cxx_objects>)
-  set(cxx_shared_sources $<TARGET_OBJECTS:cxx_objects>)
-endif()
+split_list(LIBCXX_COMPILE_FLAGS)
+split_list(LIBCXX_LINK_FLAGS)
 
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
-  add_library(cxx_shared SHARED ${cxx_shared_sources})
+  add_library(cxx_shared SHARED ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
   if(COMMAND llvm_setup_rpath)
     llvm_setup_rpath(cxx_shared)
   endif()
   target_link_libraries(cxx_shared PRIVATE ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
     PROPERTIES
+      COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
       VERSION       "${LIBCXX_ABI_VERSION}.0"
       SOVERSION     "${LIBCXX_ABI_VERSION}"
   )
+  cxx_set_common_defines(cxx_shared)
+
   list(APPEND LIBCXX_BUILD_TARGETS "cxx_shared")
   if (LIBCXX_INSTALL_SHARED_LIBRARY)
     list(APPEND LIBCXX_INSTALL_TARGETS "cxx_shared")
   endif()
   if(WIN32 AND NOT MINGW AND NOT "${CMAKE_HOST_SYSTEM_NAME}" STREQUAL "Windows")
     # Since we most likely do not have a mt.exe replacement, disable the
     # manifest bundling.  This allows a normal cmake invocation to pass which
     # will attempt to use the manifest tool to generate the bundled manifest
     set_target_properties(cxx_shared PROPERTIES
                           APPEND_STRING PROPERTY LINK_FLAGS " /MANIFEST:NO")
   endif()
 endif()
 
 # Build the static library.
 if (LIBCXX_ENABLE_STATIC)
-  add_library(cxx_static STATIC ${cxx_static_sources})
+  add_library(cxx_static STATIC ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
   target_link_libraries(cxx_static PRIVATE ${LIBCXX_LIBRARIES})
   set(CMAKE_STATIC_LIBRARY_PREFIX "lib")
   set_target_properties(cxx_static
     PROPERTIES
+      COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
   )
+  cxx_set_common_defines(cxx_shared)
+
+  if (LIBCXX_HERMETIC_STATIC_LIBRARY)
+    append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
+    append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
+    target_compile_options(cxx_static PRIVATE ${CXX_STATIC_LIBRARY_FLAGS})
+    target_compile_definitions(cxx_static PRIVATE _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
+  endif()
+
   list(APPEND LIBCXX_BUILD_TARGETS "cxx_static")
   if (LIBCXX_INSTALL_STATIC_LIBRARY)
     list(APPEND LIBCXX_INSTALL_TARGETS "cxx_static")
   endif()
   # Attempt to merge the libc++.a archive and the ABI library archive into one.
   if (LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY)
     set(MERGE_ARCHIVES_SEARCH_PATHS "")
     if (LIBCXX_CXX_ABI_LIBRARY_PATH)