[libc++] Make __num_get_float hidden

[smeenai]

Summary

It's an internal function and shouldn't be exported. It's also a source
of discrepancy in the published ABI list; these symbols aren't exported
for me on CentOS 7 or Ubuntu 16.04, leading to spurious check-cxx-abilist
failures.

[smeenai]

I'm having some second thoughts about this. Visibility for template functions makes my head spin :/ Is there a general policy we've been following for these? I didn't find much just scanning through other definitions.

[EricWF]

In D27153#606296, @smeenai wrote:

I'm having some second thoughts about this. Visibility for template functions makes my head spin :/ Is there a general policy we've been following for these? I didn't find much just scanning through other definitions.

I would suggest adding the inline keyword so that implicit instantiation are marked hidden when -fvisibility-inlines-hidden. That should be sufficient to hide the implicit instantiations in libc++.so.

[smeenai]

In D27153#613140, @EricWF wrote:

In D27153#606296, @smeenai wrote:

I'm having some second thoughts about this. Visibility for template functions makes my head spin :/ Is there a general policy we've been following for these? I didn't find much just scanning through other definitions.

I would suggest adding the inline keyword so that implicit instantiation are marked hidden when -fvisibility-inlines-hidden. That should be sufficient to hide the implicit instantiations in libc++.so.

I thought about that as well. It's slightly less intrusive, since it would only change the visibility for clients using -fvisibility-inlines-hidden. It also seems like a more roundabout way of going about hiding these symbols though, and we definitely don't want the compiler using the inline as a hint to actually perform inlining for this particular function (from what I understand, inline is mostly ignored as far as inlining goes, but it might have some effect on the compiler's cost models or something).

I feel like, if the goal is to have these symbols hidden (and I think they should be), annotating the visibility explicitly is a better approach than marking as inline. What are your thoughts on that?

[smeenai]

Pinging above question.

[smeenai]

Ping.

[EricWF]

LGTM.

include/locale

 
 template <>
 inline _LIBCPP_INLINE_VISIBILITY
 long double __do_strtod<long double>(const char* __a, char** __p2) {
     return strtold_l(__a, __p2, _LIBCPP_GET_C_LOCALE);
 }
 
 template <class _Tp>
+_LIBCPP_HIDDEN
 _Tp
 __num_get_float(const char* __a, const char* __a_end, ios_base::iostate& __err)
 {
     if (__a != __a_end)
     {
         typename remove_reference<decltype(errno)>::type __save_errno = errno;
         errno = 0;
         char *__p2;

lib/abi/x86_64-unknown-linux-gnu.abilist

 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__114collate_bynameIwED0Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__114collate_bynameIwED1Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__114collate_bynameIwED2Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__114error_categoryC2Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__114error_categoryD0Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__114error_categoryD1Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__114error_categoryD2Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__get_classnameEPKcb'}
-{'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__num_get_floatIdEET_PKcS3_Rj'}
-{'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__num_get_floatIeEET_PKcS3_Rj'}
-{'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__num_get_floatIfEET_PKcS3_Rj'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__thread_struct25notify_all_at_thread_exitEPNS_18condition_variableEPNS_5mutexE'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__thread_struct27__make_ready_at_thread_exitEPNS_17__assoc_sub_stateE'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__thread_structC1Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__thread_structC2Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__thread_structD1Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115__thread_structD2Ev'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE10pubseekoffExNS_8ios_base7seekdirEj'}
 {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__115basic_streambufIcNS_11char_traitsIcEEE10pubseekposENS_4fposI11__mbstate_tEEj'}