Message ID | a9630c4c-1df6-4dd5-f7e1-3d63c2e1f34d@gmail.com |
---|---|
State | New |
Headers | show |
Series | Fix <format> in _GLIBCXX_INLINE_VERSION mode | expand |
On Sat, 19 Nov 2022 at 13:03, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > Without this qualification I have this in _GLIBCXX_INLINE_VERSION mode: > > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: > note: candidate: 'template<class _CharT> bool std::__9::isxdigit(_CharT, > const locale&)' > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: > note: template argument deduction/substitution failed: > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1540: > note: candidate expects 2 arguments, 1 provided > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1630: > error: no matching function for call to 'isxdigit(const > std::__9::basic_string_view<char>::value_type&)' > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: > note: candidate: 'template<class _CharT> bool std::__9::isxdigit(_CharT, > const locale&)' > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: > note: template argument deduction/substitution failed: > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1630: > note: candidate expects 2 arguments, 1 provided > compiler exited with status 1 > FAIL: 17_intro/headers/c++2020/all_attributes.cc (test for excess errors) > Excess errors: > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1540: > error: no matching function for call to 'isxdigit(const > std::__9::basic_string_view<char>::value_type&)' > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1630: > error: no matching function for call to 'isxdigit(const > std::__9::basic_string_view<char>::value_type&)' > > It sounds like the most reasonable fix as this is how toupper is being > called. I think the real problem is that include/c_global/cctype is missing the NAMESPACE_VERSION macros. All declarations of std::isxdigit etc should be in the same namespace, precisely so we don't need to do this. > > libstdc++: Add missing std qualification on isxdigit calls > > libstdc++-v3/ChangeLog > > * include/std/format: Add std qualification on isxdigit calls. > > Ok to commit ? Yes, OK.
On 19/11/22 14:11, Jonathan Wakely wrote: > On Sat, 19 Nov 2022 at 13:03, François Dumont via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: >> Without this qualification I have this in _GLIBCXX_INLINE_VERSION mode: >> >> /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: >> note: candidate: 'template<class _CharT> bool std::__9::isxdigit(_CharT, >> const locale&)' >> /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: >> note: template argument deduction/substitution failed: >> /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1540: >> note: candidate expects 2 arguments, 1 provided >> /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1630: >> error: no matching function for call to 'isxdigit(const >> std::__9::basic_string_view<char>::value_type&)' >> /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: >> note: candidate: 'template<class _CharT> bool std::__9::isxdigit(_CharT, >> const locale&)' >> /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: >> note: template argument deduction/substitution failed: >> /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1630: >> note: candidate expects 2 arguments, 1 provided >> compiler exited with status 1 >> FAIL: 17_intro/headers/c++2020/all_attributes.cc (test for excess errors) >> Excess errors: >> /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1540: >> error: no matching function for call to 'isxdigit(const >> std::__9::basic_string_view<char>::value_type&)' >> /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1630: >> error: no matching function for call to 'isxdigit(const >> std::__9::basic_string_view<char>::value_type&)' >> >> It sounds like the most reasonable fix as this is how toupper is being >> called. > I think the real problem is that include/c_global/cctype is missing > the NAMESPACE_VERSION macros. > > All declarations of std::isxdigit etc should be in the same namespace, > precisely so we don't need to do this. Didn't you want to fix it this way then ? To be honest I was a little bit lost by this code: #if !__has_builtin(__builtin_toupper) # include <cctype> #endif Looks like cctype is included only for toupper, why not for isxdigit ? > >> libstdc++: Add missing std qualification on isxdigit calls >> >> libstdc++-v3/ChangeLog >> >> * include/std/format: Add std qualification on isxdigit calls. >> >> Ok to commit ? > Yes, OK. Committed.
On Sun, 20 Nov 2022, 20:45 François Dumont, <frs.dumont@gmail.com> wrote: > On 19/11/22 14:11, Jonathan Wakely wrote: > > On Sat, 19 Nov 2022 at 13:03, François Dumont via Libstdc++ > > <libstdc++@gcc.gnu.org> wrote: > >> Without this qualification I have this in _GLIBCXX_INLINE_VERSION mode: > >> > >> > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: > >> note: candidate: 'template<class _CharT> bool std::__9::isxdigit(_CharT, > >> const locale&)' > >> > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: > >> note: template argument deduction/substitution failed: > >> > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1540: > >> note: candidate expects 2 arguments, 1 provided > >> > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1630: > >> error: no matching function for call to 'isxdigit(const > >> std::__9::basic_string_view<char>::value_type&)' > >> > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: > >> note: candidate: 'template<class _CharT> bool std::__9::isxdigit(_CharT, > >> const locale&)' > >> > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/locale_facets.h:2649: > >> note: template argument deduction/substitution failed: > >> > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1630: > >> note: candidate expects 2 arguments, 1 provided > >> compiler exited with status 1 > >> FAIL: 17_intro/headers/c++2020/all_attributes.cc (test for excess > errors) > >> Excess errors: > >> > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1540: > >> error: no matching function for call to 'isxdigit(const > >> std::__9::basic_string_view<char>::value_type&)' > >> > /home/fdt/dev/gcc/build_versioned_ns/x86_64-pc-linux-gnu/libstdc++-v3/include/format:1630: > >> error: no matching function for call to 'isxdigit(const > >> std::__9::basic_string_view<char>::value_type&)' > >> > >> It sounds like the most reasonable fix as this is how toupper is being > >> called. > > I think the real problem is that include/c_global/cctype is missing > > the NAMESPACE_VERSION macros. > > > > All declarations of std::isxdigit etc should be in the same namespace, > > precisely so we don't need to do this. > > Didn't you want to fix it this way then ? > > To be honest I was a little bit lost by this code: > > #if !__has_builtin(__builtin_toupper) > # include <cctype> > #endif > > Looks like cctype is included only for toupper, why not for isxdigit ? > The idea was to only include it when needed for clang. But of course it's already included by <locale> and so that check is pointless. I think we might want to use the built-in for isxdigit as well, because the built-in ignores the locale which is what we want here. Or we should just replace toupper and isxdigit with locale-independent equivalents. > > > >> libstdc++: Add missing std qualification on isxdigit calls > >> > >> libstdc++-v3/ChangeLog > >> > >> * include/std/format: Add std qualification on isxdigit > calls. > >> > >> Ok to commit ? > > Yes, OK. > > Committed. > Thanks. >
diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format index f4fc85a16d2..9f5b7bee2be 100644 --- a/libstdc++-v3/include/std/format +++ b/libstdc++-v3/include/std/format @@ -1537,7 +1537,7 @@ namespace __format if (__trailing_zeros) { - if (!isxdigit(__s[0])) + if (!std::isxdigit(__s[0])) --__sigfigs; __z = __prec - __sigfigs; } @@ -1627,7 +1627,7 @@ namespace __format { __fill_char = _CharT('0'); // Write sign before zero filling. - if (!isxdigit(__narrow_str[0])) + if (!std::isxdigit(__narrow_str[0])) { *__out++ = __str[0]; __str.remove_prefix(1);