From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.CeBiTec.Uni-Bielefeld.DE (smtp.CeBiTec.Uni-Bielefeld.DE [129.70.160.84]) by sourceware.org (Postfix) with ESMTPS id 28AC1398B82A; Thu, 21 Jan 2021 16:36:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 28AC1398B82A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=CeBiTec.Uni-Bielefeld.DE Authentication-Results: sourceware.org; spf=none smtp.mailfrom=ro@cebitec.uni-bielefeld.de Received: from localhost (localhost [127.0.0.1]) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTP id 79B7E5225; Thu, 21 Jan 2021 17:36:21 +0100 (CET) X-Virus-Scanned: amavisd-new at CeBiTec.Uni-Bielefeld.DE Received: from smtp.CeBiTec.Uni-Bielefeld.DE ([127.0.0.1]) by localhost (smtp.cebitec.uni-bielefeld.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BGjlyas10KKY; Thu, 21 Jan 2021 17:36:17 +0100 (CET) Received: from manam.CeBiTec.Uni-Bielefeld.DE (p50855c8f.dip0.t-ipconnect.de [80.133.92.143]) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTPSA id 656D8507A; Thu, 21 Jan 2021 17:36:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=CeBiTec.Uni-Bielefeld.DE; s=20200306; t=1611246977; bh=VG9oex4S7ZE6qsD2QeCap/B3ajftQARh8ECdvjdwrNQ=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=XD2VaDPX22/bNsqTpt8HKJC3mgC/SJjlxvmvDnYEWvLbmUBCwlVQxjRhUkBKf6DRY ndjXyk438v8C7U1hH2oqOimfIAunFainZtwDe+hx1Y9HI9q4TY1ojRG1mrien/KoJD ktH5TEJ/jzINSqsUSauu4oswfzKZc3DzB/AbdLWomJ3LqhDZnacGQh7hd6oWi3BCVL OnnCpl/0Hu5nMEWqzQs/n957lzB5YvsWqwPDVCoobsnLvx1XZCzdaiwxS4f7zjSaws jJIxXYrLwfH083Lp+xTct+6yIZKaZUJqs0BHI3HTimx7Xx1vmEpemsEBl0J03S0Btt JElAoPfDX4J7Q== From: Rainer Orth To: "CHIGOT, CLEMENT" Cc: libstdc++ , David Edelsohn , Jonathan Wakely , David Edelsohn via Gcc-patches Subject: Re: [PATCH] libstdc++: implement locale support for AIX References: <20210111154058.GB21410@redhat.com> <20210112152505.GE21410@redhat.com> Date: Thu, 21 Jan 2021 17:36:16 +0100 In-Reply-To: (CLEMENT CHIGOT's message of "Thu, 21 Jan 2021 12:48:17 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1.90 (usg-unix-v) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Status: No, score=-3796.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, 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: Thu, 21 Jan 2021 16:36:27 -0000 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Clement, > Here is a new version of the patch. I've tested on Linux and=C2=A0AIX. > There are still some tests failing but it starts having a good shape !=C2= =A0 > However, I have few questions: > > 1) locale.name and syscalls just a terminology nit: none of those are syscalls. > 3) POSIX 2017 and non-POSIX functions > Many of the *_l functions being used in GNU or dragonfly models aren't=20 > POSIX 2008, but mainly POSIX 2017 or like strtof_l not POSIX at all.=20 > However, there are really useful in the code, thus I've made a double=20 > implementation based on "#ifdef HAVE_". Is it ok for you ? It's not really > POSIX 2008 but more POSIX 2008 with 2017 compatibility.=20 > For the configure, I didn't find any better way to check each syscall, as= =20 > they all depend on different includes. Tell me if you have a better idea. First a general observation: there are two groups of functions you're testing for: * Pure BSD additions, not available in either POSIX.1, ISO C, or glibc: localeconv_l mbstowcs_l strtod_l strtof_l strtold_l wcsftime_l * Part of XPG7: iswctype_l strcoll_l strftime_l strxfrm_l towlower_l towupper_l wcscoll_l wcsxfrm_l wctype_l My suggestion would be not to have configure tests _GLIBCXX_HAVE_ for any of the second group at all: this is ieee_1003.1-2008, after all, so if some OS selects that clocale variant, it better implement all of those. If really need be, one could a configure check for those and error out if any is missing. This makes the code way more readable than trying to handle some hypothetical partial implementation. As for the BSD group, I suggest to have one representative configure test (for localeconv_l perhaps) and then use an appropriate name for the group as a whole. Again, this will most likely be an all-or-nothing thing. Besides, your configure tests are way too complicated: just use AC_CHECK_FUNCS doing a link test and be done with it. In a similar vein, configure.ac already has AC_CHECK_HEADERS([xlocale.h]). Rather than hardcoding the existance of the header based on the configure triple, just use the existing HAVE_XLOCALE_H. This ways, things will simply fall into place for e.g. NetBSD, OpenBSD and possibly others. > 4) ctype_configure_char.cc=20 > I've some troubles knowing what is supposed to be implemented on this fil= e.=20 > I don't really understand the part with setlocale which appears in many=20 > os. When I'm adding it, some tests start failing, some start working...=20 > Moreover, on Linux, if I understand correctly, there is some optimization= s=20 > based on classic_table(), _M_toupper and _M_tolower. Could you confirm=20 > that it's only useful on Linux ? I don't know myself. However, when trying the first version of your patch (augmented to compile on Solaris), the corresponding change to the solaris file made no difference in test results. > Feel free to try in on other OS. But I've made modifications only for AIX= and=20 > Linux, as I can test the other ones.=20 While reading through the patch, I saw that in two places you still use __DragonFly__ || __FreeBSD__ tests. For one, it's hard to tell what feature they are really about, besides they will require fiddling with e.g. for other BSDs. Please use a descriptive macro which says which difference this is about. That said, I gave the new patch a try on Solaris 11.4. To get it to compile, I had to apply two changes that I'd mentioned (without an actual patch) when commenting on the first patch: * The C99 fields of struct lconv need _LCONV_C99 to be visible for C++11. * Some ctype macros need __bitmapsize =3D 15, as the generic clocale implementation uses. With the attached patch, the code compiled using --enable-clocale=3Dieee_1003.1-2008. Compared to the augmented first patch, there are a few differences: a couple of failures went away and I've now +XPASS: 22_locale/ctype/is/wchar_t/2.cc execution test Rainer --=20 ---------------------------------------------------------------------------= -- Rainer Orth, Center for Biotechnology, Bielefeld University --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=lx7.patch diff --git a/libstdc++-v3/config/locale/ieee_1003.1-2008/ctype_members.cc b/libstdc++-v3/config/locale/ieee_1003.1-2008/ctype_members.cc --- a/libstdc++-v3/config/locale/ieee_1003.1-2008/ctype_members.cc +++ b/libstdc++-v3/config/locale/ieee_1003.1-2008/ctype_members.cc @@ -196,7 +196,7 @@ namespace std _GLIBCXX_VISIBILITY(defaul do_is(mask __m, char_type __c) const { bool __ret = false; - const size_t __bitmasksize = 11; + const size_t __bitmasksize = 15; #ifndef _GLIBCXX_HAVE_ISWCTYPE_L __c_locale __old = uselocale((locale_t)_M_c_locale_ctype); #endif @@ -227,7 +227,7 @@ namespace std _GLIBCXX_VISIBILITY(defaul #endif for (;__lo < __hi; ++__vec, ++__lo) { - const size_t __bitmasksize = 11; + const size_t __bitmasksize = 15; mask __m = 0; for (size_t __bitcur = 0; __bitcur <= __bitmasksize; ++__bitcur) #ifdef _GLIBCXX_HAVE_ISWCTYPE_L @@ -345,7 +345,7 @@ namespace std _GLIBCXX_VISIBILITY(defaul __j < sizeof(_M_widen) / sizeof(wint_t); ++__j) _M_widen[__j] = btowc(__j); - for (size_t __k = 0; __k <= 11; ++__k) + for (size_t __k = 0; __k <= 15; ++__k) { _M_bit[__k] = static_cast(_ISbit(__k)); _M_wmask[__k] = _M_convert_to_wmask(_M_bit[__k]); diff --git a/libstdc++-v3/config/locale/ieee_1003.1-2008/monetary_members.cc b/libstdc++-v3/config/locale/ieee_1003.1-2008/monetary_members.cc --- a/libstdc++-v3/config/locale/ieee_1003.1-2008/monetary_members.cc +++ b/libstdc++-v3/config/locale/ieee_1003.1-2008/monetary_members.cc @@ -29,6 +29,8 @@ // Written by Benjamin Kosnik // Modified for DragonFly by John Marino +// Solaris 11.4 doesn't make C99 members of struct lconv visible for C++11 +// without this. #include #include --=-=-=--