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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
