[libc++] Clarify how we pick the typeinfo comparison

This commit makes it clear that the typeinfo comparison implementation
is automatically selected by default, and that the CMake option only
overrides the value. This has been a source of confusion and bugs ever
since we've introduced complexity in that area, so I'm trying to simplify
it while still allowing for some control on the implementation.

Differential Revision: https://reviews.llvm.org/D91574

libcxx/CMakeLists.txt

@@ -175,14 +175,17 @@ option(LIBCXX_ABI_UNSTABLE "Unstable ABI of libc++." OFF)
 option(LIBCXX_ABI_FORCE_ITANIUM "Ignore auto-detection and force use of the Itanium ABI.")
 option(LIBCXX_ABI_FORCE_MICROSOFT "Ignore auto-detection and force use of the Microsoft ABI.")
 
+set(LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION "default" CACHE STRING
+  "Override the implementation to use for comparing typeinfos. By default, this
+   is detected automatically by the library, but this option allows overriding
+   which implementation is used unconditionally.
 
-set(LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION "" CACHE STRING "The implementation of typeinfo comparison to use.")
-set(TYPEINFO_COMPARISON_VALUES ";1;2")
-set_property(CACHE LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION PROPERTY STRINGS ${TYPEINFO_COMPARISON_VALUES})
-list(FIND TYPEINFO_COMPARISON_VALUES "${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}" IS_VALID_DEFAULT)
-if (${IS_VALID_DEFAULT} EQUAL -1)
+   See the documentation in <libcxx/include/typeinfo> for details on what each
+   value means.")
+set(TYPEINFO_COMPARISON_VALUES "default;1;2;3")
+if (NOT ("${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}" IN_LIST TYPEINFO_COMPARISON_VALUES))
   message(FATAL_ERROR "Value '${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}' is not a valid value for
-          LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION")
+                       LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION")
 endif()
 
 option(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT "Enable per TU ABI insulation by default. To be used by vendors." OFF)
@@ -841,10 +844,9 @@ 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 (NOT LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION STREQUAL "")
+if (NOT LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION STREQUAL "default")
   config_define("${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}" _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION)
 endif()
-
 config_define_if(LIBCXX_HAS_PTHREAD_API _LIBCPP_HAS_THREAD_API_PTHREAD)
 config_define_if(LIBCXX_HAS_EXTERNAL_THREAD_API _LIBCPP_HAS_THREAD_API_EXTERNAL)
 config_define_if(LIBCXX_HAS_WIN32_THREAD_API _LIBCPP_HAS_THREAD_API_WIN32)

libcxx/cmake/caches/Apple.cmake

@@ -7,7 +7,6 @@ set(LIBCXX_ABI_VERSION "1" CACHE STRING "")
 set(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
 set(LIBCXX_ENABLE_STATIC OFF CACHE BOOL "")
 set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "")
-set(LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION "1" CACHE STRING "")
 set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
 set(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "")
 set(LIBCXX_ENABLE_DEBUG_MODE_SUPPORT OFF CACHE BOOL "")

libcxx/docs/BuildingLibcxx.rst

@@ -378,26 +378,6 @@ The following options allow building libc++ for a different ABI version.
   See ``include/__config`` for the list of ABI macros.
 
 
-.. option:: LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION
-
-  **Default**: ``None``, which lets the library figure out which implementation
-  to use based on the object format.
-
-  This setting defines what implementation to use for comparing typeinfo objects.
-  There are two main implementations, which differ on whether we make the assumption
-  that type info names for a type have been fully merged are unique across the entire
-  program. This may not be the case for libraries built with ``-Bsymbolic`` or due to
-  compiler or linker bugs (Ex. llvm.org/PR37398).
-
-
-  When the value is set to ``1``, we assume that typeinfos are unique across the
-  whole program, and typeinfo comparisons compare only the pointer value.
-
-  When the value is set to ``2``, we do not assume that typeinfos are unique across
-  the whole program. We first compare the pointers, and then use ``strcmp`` on the
-  typeinfo names as a fallback.
-
-
 .. _LLVM-specific variables:
 
 LLVM-specific options

libcxx/include/__config

@@ -755,16 +755,6 @@ typedef __char32_t char32_t;
 #  endif
 #endif
 
-#ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
-#  ifdef _LIBCPP_OBJECT_FORMAT_COFF // Windows binaries can't merge typeinfos.
-#    define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 2
-#  else
-     // TODO: This isn't strictly correct on ELF platforms due to llvm.org/PR37398
-     // And we should consider defaulting to OFF.
-#    define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 1
-#  endif
-#endif
-
 #ifndef _LIBCPP_HIDE_FROM_ABI
 #  if _LIBCPP_HIDE_FROM_ABI_PER_TU
 #    define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_INTERNAL_LINKAGE

libcxx/include/__config_site.in

@@ -28,9 +28,7 @@
 #cmakedefine _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS
 #cmakedefine _LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS
 #cmakedefine _LIBCPP_NO_VCRUNTIME
-#ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
 #cmakedefine _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION @_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION@
-#endif
 #cmakedefine _LIBCPP_ABI_NAMESPACE @_LIBCPP_ABI_NAMESPACE@
 #cmakedefine _LIBCPP_HAS_PARALLEL_ALGORITHMS
 #cmakedefine _LIBCPP_HAS_NO_RANDOM_DEVICE

libcxx/include/typeinfo

@@ -142,7 +142,7 @@ public:
 // comparison is equal.
 // -------------------------------------------------------------------------- //
 //                          NonUniqueARMRTTIBit
-// (selected on ARM64 regardless of _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION)
+//               (_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION = 3)
 // -------------------------------------------------------------------------- //
 // This implementation of type_info does not assume always a unique copy of
 // the RTTI for a given type inside a program. It packs the pointer to the
@@ -161,6 +161,24 @@ public:
 // the pointer when it constructs the type_info, depending on whether it can
 // guarantee uniqueness for that specific type_info.
 
+// This value can be overriden in the __config_site. When it's not overriden,
+// we pick a default implementation based on the platform here.
+#ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
+
+  // Windows binaries can't merge typeinfos, so use the NonUnique implementation.
+# ifdef _LIBCPP_OBJECT_FORMAT_COFF
+#   define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 2
+
+  // On arm64 on Apple platforms, use the special NonUniqueARMRTTIBit implementation.
+# elif defined(__APPLE__) && defined(__LP64__) && !defined(__x86_64__)
+#   define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 3
+
+  // On all other platforms, assume the Itanium C++ ABI and use the Unique implementation.
+# else
+#   define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 1
+# endif
+#endif
+
 struct __type_info_implementations {
   struct __string_impl_base {
     typedef const char* __type_name_t;
@@ -258,12 +276,12 @@ struct __type_info_implementations {
   };
 
   typedef
-#if defined(__APPLE__) && defined(__LP64__) && !defined(__x86_64__)
-    __non_unique_arm_rtti_bit_impl
-#elif _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 1
+#if _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 1
     __unique_impl
 #elif _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 2
     __non_unique_impl
+#elif _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 3
+    __non_unique_arm_rtti_bit_impl
 #else
 #   error invalid configuration for _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
 #endif