Differential D52240
Partial Fix for PR#38964 Authored by mclow.lists on Sep 18 2018, 9:37 AM.
Details In https://llvm.org/r336132, I broke <string> when compiled with GCC in -std=c++03 This fixes part of this, by wrapping the template constraints in an #ifndef _LIBCPP_HAS_NO_DEDUCTION_GUIDES block - since these ones are only necessary when using template deduction guides (implicit or explicit) I'm a bit concerned about whether or not this moves stuff into/out of the dylib (since we instantiate basic_string there)
Diff Detail Event TimelineComment Actions An alternate solution would be to define _LIBCPP_HAS_NO_DEFAULT_TEMPLATE_ARGS_IN_FUNCTIONS and use that. Comment Actions Built with mkdir build && cd build && cmake -G Ninja .. && ninja, the resulting libc++.so.1 has the following additional symbols: std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(char const*) std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(char const*, std::__1::allocator<char> const&) std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(unsigned long, char, std::__1::allocator<char> const&) std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(char const*) std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(char const*, std::__1::allocator<char> const&) std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(unsigned long, char, std::__1::allocator<char> const&) std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >::basic_string(wchar_t const*) std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >::basic_string(wchar_t const*, std::__1::allocator<wchar_t> const&) std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >::basic_string(unsigned long, wchar_t, std::__1::allocator<wchar_t> const&) std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >::basic_string(wchar_t const*) std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >::basic_string(wchar_t const*, std::__1::allocator<wchar_t> const&) std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >::basic_string(unsigned long, wchar_t, std::__1::allocator<wchar_t> const&) So these are now instantiated in the .so file. This is on FreeBSD, but I assume it will be the same on macOS and Linux; I can't check at the moment. I also still get some errors when compiling <string> with g++ in c++03 mode: $ make echo "#include <string>" | g++ -std=c++03 -nostdinc++ -isystem /share/dim/src/libcxx/trunk/include -x c++ -c - -o - In file included from <stdin>:1: /share/dim/src/libcxx/trunk/include/string:850:75: error: default template arguments may not be used in function templates without -std=c++11 or -std=gnu++11 const allocator_type& __a = allocator_type()); ^ /share/dim/src/libcxx/trunk/include/string:854:45: error: default template arguments may not be used in function templates without -std=c++11 or -std=gnu++11 explicit basic_string(const _Tp& __t); ^ /share/dim/src/libcxx/trunk/include/string:858:72: error: default template arguments may not be used in function templates without -std=c++11 or -std=gnu++11 explicit basic_string(const _Tp& __t, const allocator_type& __a); ^ /share/dim/src/libcxx/trunk/include/string:881:43: error: default template arguments may not be used in function templates without -std=c++11 or -std=gnu++11 basic_string& operator=(const _Tp& __t) ^ *** Error code 1 Stop. make: stopped in /share/dim/src/llvm/bugs/pr38964 Comment Actions Marshall, what's the error we get with GCC in C++03 mode that this patch is fixing? Is there a bug.llvm.org associated to that? Comment Actions I'm sorry -- I didn't see it. @dim:
I don't think that correct. All the functions are marked with _LIBCPP_INLINE_VISIBILITY, which means they are not exported from the dylib. At least that should be true on macOS. Comment Actions Marshall, can you confirm that the change does not break check-cxx-abilist on OS X. I'm fine with the change if that's not broken. Comment Actions Hmm, I tried on macOS 10.13.6 with Xcode 9.4.1 and Apple LLVM version 9.1.0 (clang-902.0.39.2), using stock libc++ r342744, but that already failed: ninja: Entering directory `/Users/dim/obj/llvm-342744-trunk-darwin17-x86_64-ninja-rel-1'
[1/1] Testing ABI compatibility...
FAILED: projects/libcxx/lib/abi/CMakeFiles/check-cxx-abilist
cd /Users/dim/obj/llvm-342744-trunk-darwin17-x86_64-ninja-rel-1/projects/libcxx/lib/abi && /Users/dim/tmp/llvm/projects/libcxx/utils/sym_diff.py --only-stdlib-symbols --strict /Users/dim/tmp/llvm/projects/libcxx/lib/abi/x86_64-apple-darwin.v1.abilist /Users/dim/obj/llvm-342744-trunk-darwin17-x86_64-ninja-rel-1/lib/libc++.1.dylib
Symbol added: __ZNSt3__16__itoa8__u32toaEjPc
{'type': 'FUNC', 'is_defined': True, 'name': '__ZNSt3__16__itoa8__u32toaEjPc'}
Symbol added: __ZNSt3__16__itoa8__u64toaEyPc
{'type': 'FUNC', 'is_defined': True, 'name': '__ZNSt3__16__itoa8__u64toaEyPc'}
Summary
Added: 2
Removed: 0
Changed: 0
Symbols added.So I assume the ABI is already broken in some way... But on a more positive note, applying this review did not change the check-cxx-abilist output, so I guess for macOS it should be okay? @emaste we should really try to also get the check-cxx-abilist target working for FreeBSD! Comment Actions Yes, I don't have a Mac to test so I'm reluctant to add those symbols when committing. Anyone who have a Mac can just add it. Comment Actions This looks OK to me. But I'm not in love with using _LIBCPP_HAS_NO_DEDUCTION_GUIDES to check for the availability of default template arguments. |