Re: [PATCH 2/3]: C++20 P0482R6 and C2X N2653: Implement mbrtoc8, c8rtomb, char8_t

[Tom Honermann <tom@honermann.net>]

On 6/11/21 12:28 PM, Joseph Myers wrote:
> On Fri, 11 Jun 2021, Tom Honermann via Libc-alpha wrote:
>
>> 1) Enabling pre-C2x compatibility with C++20.
> Where a feature in a header should be enabled for a particular C++
> version, we can have an internal __GLIBC_USE (CXX20) which is enabled
> based on the value of __cplusplus, just like the existing __USE_ISOCXX11.
> (Though that doesn't make much practical difference until we provide a way
> for C++ library headers to get all the features they need without defining
> _GNU_SOURCE.)
>
> You'd then have
>
> #if defined __USE_GNU || __GLIBC_USE (CXX20)
>
> in uchar.h, with appropriate !defined __cpp_char8_t for defining the
> typedef itself.  And the defined __USE_GNU would become __GLIBC_USE
> (ISOC2X) if the feature is accepted for C2x.

Sounds good for C.

For C++ though, the declarations of mbrtoc8() and c8rtomb() need to be 
aligned with libstdc++ expectations since they are redeclared in the std 
namespace if available.  The corresponding libstdc++ patch 
<https://gcc.gnu.org/pipermail/libstdc++/2021-June/052685.html> probes 
whether these functions are available through an autoconf probe that 
runs g++ -fchar8_t; the redeclarations are then dependent only on 
whether char8_t support is enabled (for C++, the corresponding library 
support is dependent on the core language changes).  Restricting the 
declarations as you suggest would result in compilation errors for 
invocations like g++ -std=c++17 -fchar8_t and g++ -std=c++2x -fchar8_t.  
The char8_t dialect already exists for C++, and I think the glibc 
changes should align with that for C++.  I think it is fine if the new 
functions are declared for modes like g++ -std=gnu++17; there is no need 
to disable them with -fno-char8_t.  I therefore suggest:

#if defined __USE_GNU || /* __GLIBC_USE (ISOC2X) || */ defined __cpp_char8_t

>
>> 2) Enabling conditional code sensitive to the change of type of u8 string
>> literals.
> That might be a reason for a compiler option (I don't think it's a
> sufficient reason, I expect -std=c11 or -Wno-pointer-sign or pointer casts
> to suffice in practice), but it has nothing to do with the use of feature
> test macros in glibc since the library headers don't care at all what the
> type of u8 string literals is (there are no u8 string literals anywhere in
> glibc).
That's fair.  I think Jakub's suggestion on gcc-patches to use 
__CHAR8_TYPE__ to conditionalize sensitive code suffices.
>
>> 3) Avoiding clashes with existing uses of the char8_t identifier in code that
>> may be compiled with _GNU_SOURCE.
> It's a universal feature in glibc that all non-obsolescent,
> non-conflicting features are enabled with _GNU_SOURCE, including features
> from future standard versions - all feature test macros are subsets of
> _GNU_SOURCE, except where the features actually conflict in some way (a
> few cases where a POSIX function has semantics incompatible with a GNU
> function of the same name), and features from future standards with no
> feature test macro support in glibc (e.g. some features proposed for the
> next POSIX version) are also enabled by _GNU_SOURCE.
>
> So char8_t and these functions should be enabled by _GNU_SOURCE,
> regardless of what other options enable them.
>
> Users of _GNU_SOURCE are expected to deal with possible naming conflicts
> with features added to _GNU_SOURCE.  Likewise people building with
> -std=gnu2x / -std=c2x, which would enable these declarations if char8_t
> and these functions are added to C2x, or users of _ISOC2X_SOURCE (which
> isn't expected to be used much).
Ok, great.  I wasn't aware that _GNU_SOURCE was intended to be this 
inconclusive.
>
>> I agree that this is a narrow feature, but I think the feature test macro is
>> beneficial for compatibility reasons.  Assuming the proposal is adopted for
>> C23, some programmers may need to opt-out of the feature by compiling with
>> -fno-char8_t during a migration period; as some C++20 projects have done.
> We don't provide a way to opt out of any other library additions when
> compiling in C2x mode; compiling with -std=c2x or -std=gnu2x or
> -D_GNU_SOURCE unconditionally enables all other C2x additions in headers
> (including e.g. the iszero macro, although the issues seen with that were
> mainly for C++ code and we addressed that by making it a template instead
> of a macro for C++), compiling with e.g. -std=c11 or -std=gnu11 (and not
> using _GNU_SOURCE) is how you opt out of all such additions at the same
> time.  Likewise for previous C standard versions.  And _GNU_SOURCE is the
> universal way to opt in to features that might or might not be in a future
> standard version.  If such a feature has its own TS, we may support the
> __STDC_WANT_* macros from such a TS, but that's not a basis for creating
> our own feature test macro for a handful of new identifiers.

Perfect.

Per this and prior messages, I'll review the patch series as follows:

 1. Remove uses of the _CHAR8_T_SOURCE macro.
 2. Remove the comments added for __GLIBC_USE_ISOC2X,
    __GLIBC_USE_DEPRECATED_GETS, and __GLIBC_USE_DEPRECATED_SCANF.
 3. Remove the __GLIBC_USE_CHAR8_T macro.
 4. Conditionalize the char8_t, mbrtoc8() and c8rtomb() declarations on
    __USE_GNU and __cpp_char8_t (and perhaps __GLIBC_USE (ISOC2X)
    eventually).
 5. Transition the new tests to the support/test-driver.c driver and use
    the support/check.h interfaces.
 6. Correct the copyright statements.
 7. Remove the "contributed by" comments.

If I've forgotten anything, please let me know.

Thank you for the thorough review!

Tom.