From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by sourceware.org (Postfix) with ESMTPS id 85D82388A03C; Mon, 10 Jan 2022 21:38:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 85D82388A03C Received: by mail-wr1-x42b.google.com with SMTP id s1so29337959wra.6; Mon, 10 Jan 2022 13:38:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VlfTEnLOT8kcohe43HuojTdc4QkuqgnREk5KR9n5g0A=; b=kYEj7GZhfM6Ha0WvT09sIszdLaD/L6utwv+cwUXtTX4OtYnXNNem/Edzoxgqgx3914 pWQVG1wUrIiN3Y7BYkKns/6Ptlbd61y7jZBLgvW/RydlM3QM+rR2xq2zuU1QZTo0xgLa c+IqvVU44cYHiGw8IvvySAPWYWY9jrxx0qTxuCSLEPLou+tDBmZlxLgGSE39Gfs6eRCr /jBY8oaJfBep1EGGrvbr8F8zmwiySS9hGOaHqBhnaymavK4QoFo2ouImPrFmV+yZq/mb Lr8mqG6w0kWtrpYs2hc41KLgMWesIc6BZO7pdd7KNXfAffJEf674VIEvsw4fbQsCGm0q 9RzA== X-Gm-Message-State: AOAM5301cJN7d84/sHTFFC3F9UCvfFH9C3LAP+tAK3fucHHFx/AA3iCi 8FE+UIR+gaGytUNCfW9/6+cD3a+IWQO4YkDfm90= X-Google-Smtp-Source: ABdhPJw5F5Cevzu3WziG+1CJ3jRcz8UpsPgR4TYo3zXr2+0Rt+ue3p4fDB3IOZ11wQH4WNTdRv9VC5viyA866T489R8= X-Received: by 2002:a05:6000:3c8:: with SMTP id b8mr1203016wrg.152.1641850725485; Mon, 10 Jan 2022 13:38:45 -0800 (PST) MIME-Version: 1.0 References: <79037d6b-3c48-eb7b-030a-f388fb988187@honermann.net> <09fb5a3b-fd25-7852-0c62-95afe0740b6c@honermann.net> In-Reply-To: <09fb5a3b-fd25-7852-0c62-95afe0740b6c@honermann.net> From: Jonathan Wakely Date: Mon, 10 Jan 2022 21:38:34 +0000 Message-ID: Subject: Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library To: Tom Honermann Cc: Jonathan Wakely , "libstdc++@gcc.gnu.org" , gcc-patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, RCVD_IN_DNSWL_NONE, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Jan 2022 21:38:48 -0000 On Mon, 10 Jan 2022 at 21:24, Tom Honermann via Libstdc++ wrote: > > 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. Yes, it's very easy to do. Take a look at the check_effective_target_blah procs in that file, especially the later ones that use v3_check_preprocessor_condition. You can use that to define an effective target keyword for any preprocessor condition (such as the new macros you're adding). Then the test can do: // { dg-do compile { target blah } } which will make it UNSUPPORTED if the effective target proc doesn't return true. See https://gcc.gnu.org/onlinedocs/gccint/Selectors.html#Selectors for the docs on target selectors. I'm just not sure it's worth adding a new keyword for just two tests. > > > > 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. OK, thanks.