| Message ID | 44095163-84f6-8d68-9195-eecb7d5cfbe1@gmail.com |
|---|---|
| State | New |
| Headers | show |
Some quick comments before I go offline ... On 21/10/16 21:21 +0200, François Dumont wrote: >Hi > > I configured libstdc++ to use gnu-version-namespace and there are >a number of failures, see below. But none of them related to this >patch so is it ok to commit ? > >The results: > >FAIL: libstdc++-abi/abi_check > > 3709 symbols reported as added. I don't know what to think about >it. I see a gnu-versioned-namespace.ver in config/abi/pre, is it the >list of symbols to support when versioned namespace is activated ? The >list looks pretty limited. Because everything gets matched by widlcards and added to a single symbol version. I just ignore abi-check for the versioned namespace, since it's explicitly not ABI compatible. Does the test even work for the versioned namespace or does it only use the normal baselines? >FAIL: 18_support/headers/limits/synopsis.cc (test for excess errors) >FAIL: 20_util/headers/functional/synopsis.cc (test for excess errors) >FAIL: 20_util/headers/memory/synopsis.cc (test for excess errors) >FAIL: 20_util/headers/utility/synopsis.cc (test for excess errors) >FAIL: 21_strings/headers/string/synopsis.cc (test for excess errors) >FAIL: 22_locale/headers/locale/synopsis.cc (test for excess errors) >FAIL: 23_containers/headers/bitset/synopsis.cc (test for excess errors) >FAIL: 23_containers/headers/deque/synopsis.cc (test for excess errors) >FAIL: 23_containers/headers/forward_list/synopsis.cc (test for excess >errors) >FAIL: 23_containers/headers/list/synopsis.cc (test for excess errors) >FAIL: 23_containers/headers/map/synopsis.cc (test for excess errors) >FAIL: 23_containers/headers/queue/synopsis.cc (test for excess errors) >FAIL: 23_containers/headers/set/synopsis.cc (test for excess errors) >FAIL: 23_containers/headers/stack/synopsis.cc (test for excess errors) >FAIL: 23_containers/headers/vector/synopsis.cc (test for excess errors) >FAIL: 24_iterators/headers/iterator/synopsis.cc (test for excess errors) >FAIL: 24_iterators/headers/iterator/synopsis_c++11.cc (test for excess >errors) >FAIL: 24_iterators/headers/iterator/synopsis_c++14.cc (test for excess >errors) >FAIL: 24_iterators/headers/iterator/synopsis_c++17.cc (test for excess >errors) >FAIL: 26_numerics/headers/complex/synopsis.cc (test for excess errors) >FAIL: 26_numerics/headers/valarray/synopsis.cc (test for excess errors) >FAIL: 27_io/headers/fstream/synopsis.cc (test for excess errors) >FAIL: 27_io/headers/ios/synopsis.cc (test for excess errors) >FAIL: 27_io/headers/istream/synopsis.cc (test for excess errors) >FAIL: 27_io/headers/ostream/synopsis.cc (test for excess errors) >FAIL: 27_io/headers/sstream/synopsis.cc (test for excess errors) >FAIL: 27_io/headers/streambuf/synopsis.cc (test for excess errors) >FAIL: tr1/2_general_utilities/headers/functional/synopsis.cc (test for >excess errors) >FAIL: tr1/2_general_utilities/headers/memory/synopsis.cc (test for >excess errors) >FAIL: tr1/3_function_objects/headers/functional/synopsis.cc (test for >excess errors) >FAIL: tr1/4_metaprogramming/headers/type_traits/synopsis.cc (test for >excess errors) >FAIL: tr1/6_containers/headers/array/synopsis.cc (test for excess errors) >FAIL: tr1/6_containers/headers/unordered_map/synopsis.cc (test for >excess errors) >FAIL: tr1/6_containers/headers/unordered_set/synopsis.cc (test for >excess errors) > > All those failures are coming from declaration or explicit >instantiations of template types expected to be in std but being in >fact in std::__7. Should I add usage of >_GLIBCXX_BEGIN_NAMESPACE_VERSION/_GLIBCXX_END_NAMESSPACE_VERSION in >those files ? Or introduce a dg-require-no-versioned-namespace ? I've just been ignoring those failures, as the reason is known. Either of your suggestions would work, although I've been thinking we should avoid using _GLIBCXX_ macros in the tests, so they are independent of our implementation details. We could define GLIBCXX_TEST_ macros for use in the tests, and define them independently, so we could add a GLIBCXX_TEST_INLINE_NS to those tests which would add the "inline namespace __7" bit. We don't need to worry about it for now though. >FAIL: 17_intro/using_namespace_std_tr1_neg.cc (test for errors, line 65) >FAIL: 21_strings/basic_string/cons/char/7.cc (test for excess errors) >FAIL: 21_strings/basic_string/cons/wchar_t/7.cc (test for excess errors) >FAIL: 21_strings/basic_string/lwg2758.cc (test for excess errors) >FAIL: 21_strings/basic_string/modifiers/append/char/4.cc (test for >excess errors) >FAIL: 21_strings/basic_string/modifiers/append/wchar_t/4.cc (test for >excess errors) >FAIL: 21_strings/basic_string/modifiers/assign/char/4.cc (test for >excess errors) >FAIL: 21_strings/basic_string/modifiers/assign/wchar_t/4.cc (test for >excess errors) >FAIL: 21_strings/basic_string/modifiers/insert/char/3.cc (test for >excess errors) >FAIL: 21_strings/basic_string/modifiers/insert/wchar_t/3.cc (test for >excess errors) >FAIL: 21_strings/basic_string/modifiers/replace/char/7.cc (test for >excess errors) >FAIL: 21_strings/basic_string/modifiers/replace/wchar_t/7.cc (test for >excess errors) >FAIL: 21_strings/basic_string/operations/compare/char/2.cc (test for >excess errors) >FAIL: 21_strings/basic_string/operations/compare/wchar_t/2.cc (test >for excess errors) >FAIL: 21_strings/basic_string/operations/find/char/5.cc (test for >excess errors) >FAIL: 21_strings/basic_string/operations/find/wchar_t/5.cc (test for >excess errors) >FAIL: 21_strings/basic_string/operators/char/5.cc (test for excess errors) >FAIL: 21_strings/basic_string/operators/wchar_t/5.cc (test for excess >errors) >FAIL: 21_strings/basic_string_view/cons/char/1.cc (test for excess errors) >FAIL: 21_strings/basic_string_view/cons/wchar_t/1.cc (test for excess >errors) >FAIL: 21_strings/basic_string_view/inserters/char/1.cc (test for >excess errors) >FAIL: 21_strings/basic_string_view/inserters/char/2.cc (test for >excess errors) >FAIL: 21_strings/basic_string_view/inserters/char/3.cc (test for >excess errors) >FAIL: 21_strings/basic_string_view/inserters/wchar_t/1.cc (test for >excess errors) >FAIL: 21_strings/basic_string_view/inserters/wchar_t/2.cc (test for >excess errors) >FAIL: 21_strings/basic_string_view/inserters/wchar_t/3.cc (test for >excess errors) >FAIL: 21_strings/basic_string_view/operations/string_conversion/1.cc >(test for excess errors) >FAIL: 23_containers/map/modifiers/erase/abi_tag.cc scan-assembler _ZNSt3mapIiiSt4lessIiESaISt4pairIKiiEEE5eraseB5cxx11ESt17_Rb_tree_iteratorIS4_E >FAIL: 23_containers/multimap/modifiers/erase/abi_tag.cc scan-assembler _ZNSt8multimapIiiSt4lessIiESaISt4pairIKiiEEE5eraseB5cxx11ESt17_Rb_tree_iteratorIS4_E >FAIL: 23_containers/multiset/modifiers/erase/abi_tag.cc scan-assembler _ZNSt8multisetIiSt4lessIiESaIiEE5eraseB5cxx11ESt23_Rb_tree_const_iteratorIiE >FAIL: 23_containers/multiset/modifiers/erase/abi_tag.cc scan-assembler _ZNSt8multisetIiSt4lessIiESaIiEE5eraseB5cxx11ESt23_Rb_tree_const_iteratorIiES5_ >FAIL: 23_containers/set/modifiers/erase/abi_tag.cc scan-assembler _ZNSt3setIiSt4lessIiESaIiEE5eraseB5cxx11ESt23_Rb_tree_const_iteratorIiE >FAIL: 23_containers/set/modifiers/erase/abi_tag.cc scan-assembler _ZNSt3setIiSt4lessIiESaIiEE5eraseB5cxx11ESt23_Rb_tree_const_iteratorIiES5_ >FAIL: 26_numerics/complex/abi_tag.cc scan-assembler >_ZNKSt7complexIfE4realB5cxx11Ev > > Those failures are due to dual abi being disable when versioned >namespace is being used, in configure: > > if test x$enable_symvers = xgnu-versioned-namespace; then > # gnu-versioned-namespace is incompatible with the dual ABI. > enable_libstdcxx_dual_abi="no" > fi > > Should I add a dg-require-cxx11-abi ? I have a patch to do that already, but it needs a bit more work. >FAIL: 20_util/allocator/overaligned.cc (test for excess errors) >FAIL: ext/bitmap_allocator/overaligned.cc (test for excess errors) >FAIL: ext/mt_allocator/overaligned.cc (test for excess errors) >FAIL: ext/new_allocator/overaligned.cc (test for excess errors) >FAIL: ext/pool_allocator/overaligned.cc (test for excess errors) > >Failures coming from always the same undefined symbols: >/home/fdt/dev/gcc/build_git/x86_64-pc-linux-gnu/libstdc++-v3/include/ext/new_allocator.h:108: >undefined reference to `operator new(unsigned long, std::align_val_t)' > > I don't understand those failures for now but same failures >without patch so unrelated. Those new symbols need to be added to the linker script, gnu-versioned-namespace.ver >FAIL: libstdc++-prettyprinters/cxx11.cc print efl >... > >Of course many pretty printers failures. IMO should be set as >unsupported in this mode but I don't know how to do so. No, we should fix the printers (they already try to work, but have bitrotted).
On 21/10/16 21:21 +0200, François Dumont wrote: >Hi > > I configured libstdc++ to use gnu-version-namespace and there are >a number of failures, see below. But none of them related to this >patch so is it ok to commit ? Yes, OK to commit - it doesn't make the test results any worse than they were before :-) And I agree it is wrong for the containers to not be in the versioned namespace, so this fixes it - thanks for cleaning it up.
diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config index 656ef78..ebabcd5 100644 --- a/libstdc++-v3/include/bits/c++config +++ b/libstdc++-v3/include/bits/c++config @@ -358,6 +358,10 @@ namespace std namespace _GLIBCXX_STD_C { _GLIBCXX_BEGIN_NAMESPACE_VERSION # define _GLIBCXX_END_NAMESPACE_CONTAINER \ _GLIBCXX_END_NAMESPACE_VERSION } +#else +# define _GLIBCXX_STD_C std +# define _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _GLIBCXX_BEGIN_NAMESPACE_VERSION +# define _GLIBCXX_END_NAMESPACE_CONTAINER _GLIBCXX_END_NAMESPACE_VERSION #endif #ifdef _GLIBCXX_PARALLEL @@ -366,30 +370,10 @@ namespace std namespace _GLIBCXX_STD_A { _GLIBCXX_BEGIN_NAMESPACE_VERSION # define _GLIBCXX_END_NAMESPACE_ALGO \ _GLIBCXX_END_NAMESPACE_VERSION } -#endif - -#ifndef _GLIBCXX_STD_A +#else # define _GLIBCXX_STD_A std -#endif - -#ifndef _GLIBCXX_STD_C -# define _GLIBCXX_STD_C std -#endif - -#ifndef _GLIBCXX_BEGIN_NAMESPACE_ALGO -# define _GLIBCXX_BEGIN_NAMESPACE_ALGO -#endif - -#ifndef _GLIBCXX_END_NAMESPACE_ALGO -# define _GLIBCXX_END_NAMESPACE_ALGO -#endif - -#ifndef _GLIBCXX_BEGIN_NAMESPACE_CONTAINER -# define _GLIBCXX_BEGIN_NAMESPACE_CONTAINER -#endif - -#ifndef _GLIBCXX_END_NAMESPACE_CONTAINER -# define _GLIBCXX_END_NAMESPACE_CONTAINER +# define _GLIBCXX_BEGIN_NAMESPACE_ALGO _GLIBCXX_BEGIN_NAMESPACE_VERSION +# define _GLIBCXX_END_NAMESPACE_ALGO _GLIBCXX_END_NAMESPACE_VERSION #endif // GLIBCXX_ABI Deprecated
Hi I configured libstdc++ to use gnu-version-namespace and there are a number of failures, see below. But none of them related to this patch so is it ok to commit ? The results: FAIL: libstdc++-abi/abi_check 3709 symbols reported as added. I don't know what to think about it. I see a gnu-versioned-namespace.ver in config/abi/pre, is it the list of symbols to support when versioned namespace is activated ? The list looks pretty limited. FAIL: 18_support/headers/limits/synopsis.cc (test for excess errors) FAIL: 20_util/headers/functional/synopsis.cc (test for excess errors) FAIL: 20_util/headers/memory/synopsis.cc (test for excess errors) FAIL: 20_util/headers/utility/synopsis.cc (test for excess errors) FAIL: 21_strings/headers/string/synopsis.cc (test for excess errors) FAIL: 22_locale/headers/locale/synopsis.cc (test for excess errors) FAIL: 23_containers/headers/bitset/synopsis.cc (test for excess errors) FAIL: 23_containers/headers/deque/synopsis.cc (test for excess errors) FAIL: 23_containers/headers/forward_list/synopsis.cc (test for excess errors) FAIL: 23_containers/headers/list/synopsis.cc (test for excess errors) FAIL: 23_containers/headers/map/synopsis.cc (test for excess errors) FAIL: 23_containers/headers/queue/synopsis.cc (test for excess errors) FAIL: 23_containers/headers/set/synopsis.cc (test for excess errors) FAIL: 23_containers/headers/stack/synopsis.cc (test for excess errors) FAIL: 23_containers/headers/vector/synopsis.cc (test for excess errors) FAIL: 24_iterators/headers/iterator/synopsis.cc (test for excess errors) FAIL: 24_iterators/headers/iterator/synopsis_c++11.cc (test for excess errors) FAIL: 24_iterators/headers/iterator/synopsis_c++14.cc (test for excess errors) FAIL: 24_iterators/headers/iterator/synopsis_c++17.cc (test for excess errors) FAIL: 26_numerics/headers/complex/synopsis.cc (test for excess errors) FAIL: 26_numerics/headers/valarray/synopsis.cc (test for excess errors) FAIL: 27_io/headers/fstream/synopsis.cc (test for excess errors) FAIL: 27_io/headers/ios/synopsis.cc (test for excess errors) FAIL: 27_io/headers/istream/synopsis.cc (test for excess errors) FAIL: 27_io/headers/ostream/synopsis.cc (test for excess errors) FAIL: 27_io/headers/sstream/synopsis.cc (test for excess errors) FAIL: 27_io/headers/streambuf/synopsis.cc (test for excess errors) FAIL: tr1/2_general_utilities/headers/functional/synopsis.cc (test for excess errors) FAIL: tr1/2_general_utilities/headers/memory/synopsis.cc (test for excess errors) FAIL: tr1/3_function_objects/headers/functional/synopsis.cc (test for excess errors) FAIL: tr1/4_metaprogramming/headers/type_traits/synopsis.cc (test for excess errors) FAIL: tr1/6_containers/headers/array/synopsis.cc (test for excess errors) FAIL: tr1/6_containers/headers/unordered_map/synopsis.cc (test for excess errors) FAIL: tr1/6_containers/headers/unordered_set/synopsis.cc (test for excess errors) All those failures are coming from declaration or explicit instantiations of template types expected to be in std but being in fact in std::__7. Should I add usage of _GLIBCXX_BEGIN_NAMESPACE_VERSION/_GLIBCXX_END_NAMESSPACE_VERSION in those files ? Or introduce a dg-require-no-versioned-namespace ? FAIL: 17_intro/using_namespace_std_tr1_neg.cc (test for errors, line 65) FAIL: 21_strings/basic_string/cons/char/7.cc (test for excess errors) FAIL: 21_strings/basic_string/cons/wchar_t/7.cc (test for excess errors) FAIL: 21_strings/basic_string/lwg2758.cc (test for excess errors) FAIL: 21_strings/basic_string/modifiers/append/char/4.cc (test for excess errors) FAIL: 21_strings/basic_string/modifiers/append/wchar_t/4.cc (test for excess errors) FAIL: 21_strings/basic_string/modifiers/assign/char/4.cc (test for excess errors) FAIL: 21_strings/basic_string/modifiers/assign/wchar_t/4.cc (test for excess errors) FAIL: 21_strings/basic_string/modifiers/insert/char/3.cc (test for excess errors) FAIL: 21_strings/basic_string/modifiers/insert/wchar_t/3.cc (test for excess errors) FAIL: 21_strings/basic_string/modifiers/replace/char/7.cc (test for excess errors) FAIL: 21_strings/basic_string/modifiers/replace/wchar_t/7.cc (test for excess errors) FAIL: 21_strings/basic_string/operations/compare/char/2.cc (test for excess errors) FAIL: 21_strings/basic_string/operations/compare/wchar_t/2.cc (test for excess errors) FAIL: 21_strings/basic_string/operations/find/char/5.cc (test for excess errors) FAIL: 21_strings/basic_string/operations/find/wchar_t/5.cc (test for excess errors) FAIL: 21_strings/basic_string/operators/char/5.cc (test for excess errors) FAIL: 21_strings/basic_string/operators/wchar_t/5.cc (test for excess errors) FAIL: 21_strings/basic_string_view/cons/char/1.cc (test for excess errors) FAIL: 21_strings/basic_string_view/cons/wchar_t/1.cc (test for excess errors) FAIL: 21_strings/basic_string_view/inserters/char/1.cc (test for excess errors) FAIL: 21_strings/basic_string_view/inserters/char/2.cc (test for excess errors) FAIL: 21_strings/basic_string_view/inserters/char/3.cc (test for excess errors) FAIL: 21_strings/basic_string_view/inserters/wchar_t/1.cc (test for excess errors) FAIL: 21_strings/basic_string_view/inserters/wchar_t/2.cc (test for excess errors) FAIL: 21_strings/basic_string_view/inserters/wchar_t/3.cc (test for excess errors) FAIL: 21_strings/basic_string_view/operations/string_conversion/1.cc (test for excess errors) FAIL: 23_containers/map/modifiers/erase/abi_tag.cc scan-assembler _ZNSt3mapIiiSt4lessIiESaISt4pairIKiiEEE5eraseB5cxx11ESt17_Rb_tree_iteratorIS4_E FAIL: 23_containers/multimap/modifiers/erase/abi_tag.cc scan-assembler _ZNSt8multimapIiiSt4lessIiESaISt4pairIKiiEEE5eraseB5cxx11ESt17_Rb_tree_iteratorIS4_E FAIL: 23_containers/multiset/modifiers/erase/abi_tag.cc scan-assembler _ZNSt8multisetIiSt4lessIiESaIiEE5eraseB5cxx11ESt23_Rb_tree_const_iteratorIiE FAIL: 23_containers/multiset/modifiers/erase/abi_tag.cc scan-assembler _ZNSt8multisetIiSt4lessIiESaIiEE5eraseB5cxx11ESt23_Rb_tree_const_iteratorIiES5_ FAIL: 23_containers/set/modifiers/erase/abi_tag.cc scan-assembler _ZNSt3setIiSt4lessIiESaIiEE5eraseB5cxx11ESt23_Rb_tree_const_iteratorIiE FAIL: 23_containers/set/modifiers/erase/abi_tag.cc scan-assembler _ZNSt3setIiSt4lessIiESaIiEE5eraseB5cxx11ESt23_Rb_tree_const_iteratorIiES5_ FAIL: 26_numerics/complex/abi_tag.cc scan-assembler _ZNKSt7complexIfE4realB5cxx11Ev Those failures are due to dual abi being disable when versioned namespace is being used, in configure: if test x$enable_symvers = xgnu-versioned-namespace; then # gnu-versioned-namespace is incompatible with the dual ABI. enable_libstdcxx_dual_abi="no" fi Should I add a dg-require-cxx11-abi ? FAIL: 20_util/allocator/overaligned.cc (test for excess errors) FAIL: ext/bitmap_allocator/overaligned.cc (test for excess errors) FAIL: ext/mt_allocator/overaligned.cc (test for excess errors) FAIL: ext/new_allocator/overaligned.cc (test for excess errors) FAIL: ext/pool_allocator/overaligned.cc (test for excess errors) Failures coming from always the same undefined symbols: /home/fdt/dev/gcc/build_git/x86_64-pc-linux-gnu/libstdc++-v3/include/ext/new_allocator.h:108: undefined reference to `operator new(unsigned long, std::align_val_t)' I don't understand those failures for now but same failures without patch so unrelated. FAIL: 26_numerics/gcd/gcd_neg.cc (test for excess errors) FAIL: 26_numerics/lcm/lcm_neg.cc (test for excess errors) Wrong line numbers of expected static asserts. I will fix those if not already done. FAIL: libstdc++-prettyprinters/cxx11.cc print efl ... Of course many pretty printers failures. IMO should be set as unsupported in this mode but I don't know how to do so. François On 06/10/2016 18:06, Jonathan Wakely wrote: > On 01/10/16 14:22 +0200, François Dumont wrote: >> On 30/09/2016 17:40, Jonathan Wakely wrote: >>> On 29/09/16 21:59 +0200, François Dumont wrote: >>>> Hi >>>> >>>> I think _GLIBCXX_BEGIN_NAMESPACE_ALGO should default to >>>> _GLIBCXX_BEGIN_NAMESPACE_VERSION when parallel mode is not active. >>>> Same for _GLIBCXX_BEGIN_NAMESPACE_CONTAINER, no ? >>> >>> Hmm, yes, I think this is correct, otherwise we're missing the VERSION >>> namespace in normal mode. But it seems we've always been missing it >>> since those macros were introduced in GCC 4.6 so I'd like to >>> investigate the consequences for gnu-versioned-namespace more >>> carefuly. >>> >>>> * include/bits/c++config (_GLIBCXX_BEGIN_NAMESPACE_ALGO) >>>> (_GLIBCXX_END_NAMESPACE_ALGO): Default to respectively >>>> _GLIBCXX_BEGIN_NAMESPACE_VERSION and _GLIBCXX_END_NAMESPACE_VERSION >>>> when parallel mode is not active. >>>> (_GLIBCXX_BEGIN_NAMESPACE_CONTAINER, >>>> _GLIBCXX_END_NAMESPACE_CONTAINER): >>>> Likewise. >>>> >>>> Ok to commit after normal check ? Should I rebuild library with >>>> versioned namespace activated ? >>> >>> Any change affecting the NAMESPACE_VERSION macros should be tested >>> with the gnu-versioned-namespace, since those macros are only used for >>> that mode. >>> >>> But it can't be tested currently, see PR 77794. So let's wait until >>> I've fixed 77794, when we can test it. >>> >>> >> The reason I am proposing this is that if my debug algos patch is >> applied a lot of algos won't be in versionned namespace anymore. >> >> Broken for broken I might apply it even if not really tested, no ? > > It can be tested now, I've fixed PR 77794. > > >