Use __attribute__((internal_linkage)) when available.

[eugenis]

Summary

Use attribute((internal_linkage)) instead of always_inline and visibility("hidden") when it is available.

[eugenis]

Note, this breaks tuple_cat.pass.cpp test.

With -O0, replacing always_inline with internal_linkage results in less optimization being done (namely, no inlining happens). This ends up exposing

https://llvm.org/bugs/show_bug.cgi?id=23256

which is fixed by

http://reviews.llvm.org/D12502

The same failure can be reproduced in the current ToT libc++ by running this test with -O2.

This change depends on D12502.

[eugenis]

This change depends on D15404.

[rsmith]

include/__config

237

Use __internal_linkage__ here. Users are allowed to #define internal_linkage before including this header. (x4)

[EricWF]

Why does this depend on D15404?

[EricWF]

In D14411#306716, @EricWF wrote:

Why does this depend on D15404?

Woops, I meant the tuple patch but I see the other comment now. I'm curious as to how inlininging ends up affecting which overload's SFINAE are evaluated.

Drive by comment: Is the change from __attribute__((__visibility__("hidden"), __always_inline__)) to __attribute__((__internal_linkage__)) ABI compatible?

[eugenis]

In D14411#306722, @EricWF wrote:

In D14411#306716, @EricWF wrote:

Why does this depend on D15404?

Woops, I meant the tuple patch but I see the other comment now. I'm curious as to how inlininging ends up affecting which overload's SFINAE are evaluated.

As I understand, in that test we pick a default(?) constructor instead of a move(?) constructor, and end up reading uninitialized memory. Then any code change can affect the test result. Like adding -O2 does, for example.

Drive by comment: Is the change from __attribute__((__visibility__("hidden"), __always_inline__)) to __attribute__((__internal_linkage__)) ABI compatible?

I think so. I'll verify tomorrow.

[eugenis]

Depends on D15432.
Depends on D15433.

With all that change, the switch to internal_linkage attribute removes 3 symbols from the libc++ export table, all in basic_string:
insert(..., InputIterator
insert(..., ForwardIterator
replace(..., InputIterator

These are template methods of a template class. They are instantiated only in functions/methods that are marked with LIBCPP_INLINE_VISIBILITY; normally they are exported as linkonce_odr; after the internal_linkage switch they are not instantiated at all because their callers are never evaluated.

Do you think this is an ABI break?

[eugenis]

With http://reviews.llvm.org/D15434, there is no difference in libc++ export list with the switch to internal_linkage.

[EricWF]

Sorry about the long delay in reviewing this. @eugenis Are you still able/willing to proceed with this?

[eugenis]

Yes, I'd like to try.
I think this is blocked on the changes that move visibility attributes to the first declaration, right?
Also, re: cfi-commits thread for r255177, it appears that on Mac we can neither hide nor expose existing methods (i.e. if something was hidden, it can not be exported and vice versa; basically, the set of exports must stay exactly the same).

[ldionne]

I think this should be closed since the work we did on internal_linkage in 2018. @eugenis

include/__config

 # define _LIBCPP_TYPE_VIS_ONLY _LIBCPP_TYPE_VIS
 #endif
 
 #ifndef _LIBCPP_FUNC_VIS_ONLY
 # define _LIBCPP_FUNC_VIS_ONLY _LIBCPP_FUNC_VIS
 #endif
 
 #ifndef _LIBCPP_INLINE_VISIBILITY
+#if __has_attribute(internal_linkage)
+#define _LIBCPP_INLINE_VISIBILITY __attribute__ ((internal_linkage))
+#else
 #define _LIBCPP_INLINE_VISIBILITY __attribute__ ((__visibility__("hidden"), __always_inline__))
 #endif
+#endif
 
 #ifndef _LIBCPP_EXCEPTION_ABI
 #define _LIBCPP_EXCEPTION_ABI __attribute__ ((__visibility__("default")))
 #endif
 
 #ifndef _LIBCPP_ALWAYS_INLINE
-#define _LIBCPP_ALWAYS_INLINE  __attribute__ ((__visibility__("hidden"), __always_inline__))
+#if __has_attribute(internal_linkage)
+#define _LIBCPP_ALWAYS_INLINE __attribute__ ((internal_linkage))
+#else
+#define _LIBCPP_ALWAYS_INLINE __attribute__ ((__visibility__("hidden"), __always_inline__))
 #endif
+#endif
 
 #if defined(__clang__)
 
 // _LIBCPP_ALTERNATE_STRING_LAYOUT is an old name for
 // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT left here for backward compatibility.
 #if (defined(__APPLE__) && !defined(__i386__) && !defined(__x86_64__) &&       \
      !defined(__arm__)) ||                                                     \
     defined(_LIBCPP_ALTERNATE_STRING_LAYOUT)