[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__config:798
 
-// Just so we can migrate to _LIBCPP_HIDE_FROM_ABI gradually.
-#define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
-
-#ifndef _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY
-#  if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
-#    define _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY __attribute__((__visibility__("default"), __always_inline__))
+#ifdef _LIBCPP_BUILDING_LIBRARY
+#  if _LIBCPP_ABI_VERSION > 1
----------------
dexonsmith wrote:
> It looks like if you switch this to `#if !defined(...)` you can use `#elif` instead of a nested `#if`.  I think that would make the logic a bit more clear for me, but if you disagree feel free to leave it as is.
The reason I did it that way is that this structure will make it easier to add new versions in the future:

```
#ifdef _LIBCPP_BUILDING_LIBRARY
#  if _LIBCPP_ABI_VERSION > 1
#    define _LIBCPP_HIDE_FROM_ABI_AFTER_V1 _LIBCPP_HIDE_FROM_ABI
#  else
#    define _LIBCPP_HIDE_FROM_ABI_AFTER_V1
#  endif

#  if _LIBCPP_ABI_VERSION > 2
#    define _LIBCPP_HIDE_FROM_ABI_AFTER_V2 _LIBCPP_HIDE_FROM_ABI
#  else
#    define _LIBCPP_HIDE_FROM_ABI_AFTER_V2
#  endif

#else
#  define _LIBCPP_HIDE_FROM_ABI_AFTER_V1 _LIBCPP_HIDE_FROM_ABI
#  define _LIBCPP_HIDE_FROM_ABI_AFTER_V2 _LIBCPP_HIDE_FROM_ABI
#endif
```

With your suggestion, the case where we have only one ABI version is indeed clearer, but it's not as easy to add a new version (remember than if the version is `2`, both `_LIBCPP_HIDE_FROM_ABI_AFTER_V1` and `_LIBCPP_HIDE_FROM_ABI_AFTER_V2` would need to be defined, so an `elif` chain does not cut it):

```
// Hide symbols when we're not building the dylib
#if !defined(_LIBCPP_BUILDING_LIBRARY)
#  define _LIBCPP_HIDE_FROM_ABI_AFTER_V1 _LIBCPP_HIDE_FROM_ABI
// Otherwise, hide symbols depending on the ABI version used to build the dylib
#elif _LIBCPP_ABI_VERSION > 1
#  define _LIBCPP_HIDE_FROM_ABI_AFTER_V1 _LIBCPP_HIDE_FROM_ABI
#else
#  define _LIBCPP_HIDE_FROM_ABI_AFTER_V1
#endif
```

This basically needs to turn into what I've put above if we are to add a new ABI version.



Repository:
  rCXX libc++

https://reviews.llvm.org/D49914