[libcxx-commits] [PATCH] D154796: [libc++] Recategorize additional instantiations in the dylib as availability macros

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Nice! I don't know why we didn't do those using availability in the first place, it seems strictly better.



================
Comment at: libcxx/include/__availability:180
+// using that class define their own copies.
+#   define _LIBCPP_AVAILABILITY_ENABLE_BAD_FUNCTION_CALL_KEY_FUNCTION
+#   define _LIBCPP_AVAILABILITY_ENABLE_ADDITIONAL_IOSTREAM_EXPLICIT_INSTANTIATIONS_1
----------------
I would turn these conditions around instead and use `HAS_NO_XXX` like all the other availability macros.


================
Comment at: libcxx/include/__availability:181
+#   define _LIBCPP_AVAILABILITY_ENABLE_BAD_FUNCTION_CALL_KEY_FUNCTION
+#   define _LIBCPP_AVAILABILITY_ENABLE_ADDITIONAL_IOSTREAM_EXPLICIT_INSTANTIATIONS_1
+
----------------
This macro needs a separate comment.


================
Comment at: libcxx/include/__availability:328
         __attribute__((availability(watchos,strict,introduced=7.0)))
 
     // __libcpp_verbose_abort
----------------
Let's add an entry here for `_LIBCPP_AVAILABILITY_ENABLE_BAD_FUNCTION_CALL_KEY_FUNCTION`. Those are the macros you should check to determine that the dylib *did not yet* contain the key function:

```
defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ < 120000
defined(__ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__) && __ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__ < 150000
defined(__ENVIRONMENT_TV_OS_VERSION_MIN_REQUIRED__) && __ENVIRONMENT_TV_OS_VERSION_MIN_REQUIRED__ < 150000
defined(__ENVIRONMENT_WATCH_OS_VERSION_MIN_REQUIRED__) && __ENVIRONMENT_WATCH_OS_VERSION_MIN_REQUIRED__ < 80000
```



================
Comment at: libcxx/include/__config:176-181
-// For XCOFF linkers, we have problems if we see a weak hidden version of a symbol
-// in user code (like you get with -fvisibility-inlines-hidden) and then a strong def
-// in the library, so we need to always rely on the library version.
-#    if defined(_AIX)
-#      define _LIBCPP_ABI_BAD_FUNCTION_CALL_KEY_FUNCTION
-#    endif
----------------
This needs to be translated to the availability, or it'll fail on FreeBSD. FreeBSD doesn't currently enable availability markup, but that is probably needed as a pre-requisite. We could start by unconditionally pretending that all the `_LIBCPP_AVAILABILITY_XXXXXXXX` are enabled on FreeBSD, which would be equivalent to what they have today.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154796/new/

https://reviews.llvm.org/D154796