Re: [PATCH] Fix <format> in _GLIBCXX_INLINE_VERSION mode

[Jonathan Wakely <jwakely.gcc@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3445 bytes --]

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.



>