From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 1845D3853812 for ; Mon, 17 May 2021 17:14:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1845D3853812 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-569-bdwWBfrNPR-WHbzKZ06Mmw-1; Mon, 17 May 2021 13:14:52 -0400 X-MC-Unique: bdwWBfrNPR-WHbzKZ06Mmw-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 101CF1013721; Mon, 17 May 2021 17:14:51 +0000 (UTC) Received: from localhost (unknown [10.33.36.7]) by smtp.corp.redhat.com (Postfix) with ESMTP id B31DB2B4DD; Mon, 17 May 2021 17:14:50 +0000 (UTC) Date: Mon, 17 May 2021 18:14:49 +0100 From: Jonathan Wakely To: Antony Polukhin Cc: libstdc++ , gcc-patches List Subject: Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions Message-ID: References: <20210511200031.GZ3008@redhat.com> <20210512091833.GA3008@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-8.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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, 17 May 2021 17:14:55 -0000 On 12/05/21 17:16 +0100, Jonathan Wakely wrote: >On 12/05/21 18:51 +0300, Antony Polukhin via Libstdc++ wrote: >>ср, 12 мая 2021 г. в 18:38, Antony Polukhin : >>> >>>ср, 12 мая 2021 г. в 17:44, Jonathan Wakely : >>>> >>>> On 12/05/21 12:58 +0300, Antony Polukhin wrote: >>>> >ср, 12 мая 2021 г. в 12:18, Jonathan Wakely : >>>> ><...> >>>> >> Or just leave it undefined, as libc++ seems to do according to your >>>> >> comment in PR 89728: >>>> >> >>>> >> error: implicit instantiation of undefined template 'std::__1::ctype >' >>>> >> >>>> >> Was your aim to have a static_assert that gives a more descriptive >>>> >> error? We could leave it undefined in C++98 and have the static assert >>>> >> for C++11 and up. >>>> > >>>> >Leaving it undefined would be the best. It would allow SFINAE on ctype >>>> >and a compile time error is informative enough. >>>> > >>>> >However, there may be users who instantiate ctype in a >>>> >shared library without ctype template specializations in >>>> >the main executable. Making the default ctype undefined would break >>>> >their compilation: >>>> > >>>> >#include >>>> >// no ctype specialization >>>> >c = std::tolower(ThierChar{42}, locale_from_shared_library()); // OK >>>> >right now in libstdc++, fails on libc++ >>>> >>>> What I meant was leaving the partial specialization undefined, not the >>>> primary template, i.e. >>>> >>>> --- a/libstdc++-v3/include/bits/locale_facets.h >>>> +++ b/libstdc++-v3/include/bits/locale_facets.h >>>> @@ -1476,6 +1476,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>> }; >>>> #endif //_GLIBCXX_USE_WCHAR_T >>>> >>>> + template >>>> + class ctype >; >>>> + >>>> /// class ctype_byname [22.2.1.2]. >>>> template >>>> class ctype_byname : public ctype<_CharT> >>>> >>>> This makes your test fail with errors like this: >>>> >>>> In file included from /home/jwakely/gcc/12/include/c++/12.0.0/locale:40, >>>> from loc.C:1: >>>> /home/jwakely/gcc/12/include/c++/12.0.0/bits/locale_facets.h: In instantiation of 'bool std::isspace(_CharT, const std::locale&) [with _CharT = std::__cxx11::basic_string]': >>>> loc.C:16:15: required from here >>>> /home/jwakely/gcc/12/include/c++/12.0.0/bits/locale_facets.h:2600:47: error: invalid use of incomplete type 'const class std::ctype >' >>>> 2600 | { return use_facet >(__loc).is(ctype_base::space, __c); } >>>> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ >>>> >>>> But it shouldn't affect the uses of ctype. >>>> >>>> What do you think? >>> >>>Good idea. That way the compiler message points directly to the >>>misused function. >>> >>>Patch is in attachment >> >>Replaced {} with () in test to be C++98 compatible > >Looks great, thanks. > >I'll test and commit this tomorrow. Not quite "tomorrow", but it's pushed to trunk now. Thanks again!