From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp80.ord1d.emailsrvr.com (smtp80.ord1d.emailsrvr.com [184.106.54.80]) by sourceware.org (Postfix) with ESMTPS id DB04D385840E for ; Mon, 10 Jan 2022 21:23:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DB04D385840E X-Auth-ID: tom@honermann.net Received: by smtp3.relay.ord1d.emailsrvr.com (Authenticated sender: tom-AT-honermann.net) with ESMTPSA id AD43E60128; Mon, 10 Jan 2022 16:23:44 -0500 (EST) Subject: Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <79037d6b-3c48-eb7b-030a-f388fb988187@honermann.net> From: Tom Honermann Message-ID: <09fb5a3b-fd25-7852-0c62-95afe0740b6c@honermann.net> Date: Mon, 10 Jan 2022 16:23:44 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Classification-ID: a78515e7-1004-4fc9-ba29-9c0630ab3a5f-1-1 X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, HTML_MESSAGE, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Jan 2022 21:23:48 -0000 On 1/10/22 8:23 AM, Jonathan Wakely wrote: > > > On Sat, 8 Jan 2022 at 00:42, Tom Honermann via Libstdc++ > > wrote: > > This patch completes implementation of the C++20 proposal P0482R6 > [1] by > adding declarations of std::c8rtomb() and std::mbrtoc8() in > if > provided by the C library in . > > This patch addresses feedback provided in response to a previous > patch > submission [2]. > > Autoconf changes determine if the C library declares c8rtomb and > mbrtoc8 > at global scope when uchar.h is included and compiled with either > -fchar8_t or -std=c++20. New > _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T > and _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20 configuration macros > reflect the probe results. The header declares these > functions > in the std namespace only if available and the _GLIBCXX_USE_CHAR8_T > configuration macro is defined (by default it is defined if the C++20 > __cpp_char8_t feature test macro is defined) > > Patches to glibc to implement c8rtomb and mbrtoc8 have been > submitted [3]. > > New tests validate the presence of these declarations. The tests pass > trivially if the C library does not provide these functions. > Otherwise > they ensure that the functions are declared when is included > and either -fchar8_t or -std=c++20 is enabled. > > Tested on Linux x86_64. > > libstdc++-v3/ChangeLog: > > 2022-01-07  Tom Honermann  > > >         * acinclude.m4 Define config macros if uchar.h provides >         c8rtomb() and mbrtoc8(). >         * config.h.in : Re-generate. >         * configure: Re-generate. >         * include/c_compatibility/uchar.h: Declare ::c8rtomb and >         ::mbrtoc8. >         * include/c_global/cuchar: Declare std::c8rtomb and >         std::mbrtoc8. >         * include/c_std/cuchar: Declare std::c8rtomb and std::mbrtoc8. >         * testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc: >         New test. >         * > testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc: >         New test. > > > > Thanks, Tom, this looks good and I'll get it committed for GCC 12. Thank you! > > My only concern is that the new tests depend on an internal macro: > > +#if _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20 > +  using std::mbrtoc8; > +  using std::c8rtomb; > > I prefer if tests are written as "user code" when possible, and not > using our internal macros. That isn't always possible, and in this > case would require adding new effective-target keyword to > testsuite/lib/libstdc++.exp just for use in these two tests. I don't > think we should bother with that. I went with this approach solely due to my unfamiliarity with the test system. I knew there should be a way to conditionally make the test "pass" as unsupported or as an expected failure, but didn't know how to go about implementing that. I don't mind following up with an additional patch if such a change is desirable. I took a look at testsuite/lib/libstdc++.exp and it looks like it may be pretty straight forward to add effective-target support. It would probably be a good learning experience for me. I'll prototype and report back. > > I suppose strictly speaking we should not define __cpp_lib_char8_t > unless these two functions are present in libc. But I'm not sure we > want to change that now either. All of libstdc++, libc++, and MS STL have been defining __cpp_lib_char8_t despite the absence of these functions, so yeah, I don't think we want to change that. Tom.