Hi Clement, >> > 3) POSIX 2017 and non-POSIX functions >> > Many of the *_l functions being used in GNU or dragonfly models aren't >> > POSIX 2008, but mainly POSIX 2017 or like strtof_l not POSIX at all. >> > However, there are really useful in the code, thus I've made a double >> > implementation based on "#ifdef HAVE_". Is it ok for you ? It's not really >> > POSIX 2008 but more POSIX 2008 with 2017 compatibility. >> > For the configure, I didn't find any better way to check each syscall, as >> > 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_<FUNC> >> 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. > > In this case, it would be better to call it ieee_1003.1-2017 but I agree
why? I've just double-checked the OpenGroup pages: all of the functions listed as XPG7 above were part of IEEE 1003.1-2008, just some of them have Technical Corrigenda applied. IIUC IEEE 1003.1-2017 is just a revision of the -2008 standard, not a new issue (XPG8 or something). > it would be better to avoid all these #ifdef. > Some are still needed as for example only the last version of AIX have > strftime_l. Then that version doesn't conform to XPG7 and shouldn't use that clocale variant. Until we have a clearer understanding of the variation here, I'd argue that only P1003.1-2008 conforming OSes should use ieee_1003.1-2008, rather than creating an impenetrable maze of #ifdefs for all sorts of partial implementations. >> 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. > > I'm not sure this is really all-or-nothing for these. Maybe strtof_l and cie > can be grouped by. But the 3 others are really different. Linux have > wcsftime_l > but not the others. AIX avec none. BSD have all. TBH, I don't care about Linux here: it will continue to use the gnu variant anyway. Besides, since the patch will have to work on targets without wcsftime_l and the other BSD functions, I don't see any harm in not using one non-standard one of them although it's present. >> Besides, your configure tests are way too complicated: just use >> AC_CHECK_FUNCS doing a link test and be done with it. > > Sadly, you can't pass includes to AC_CHECK_FUNCS. That's why I had to do > that. I've made a first version with AC_CHECK_DECLS which allows extra > headers, but it didn't work too. I might know why though. Why would you need to? AC_CHECK_FUNCS just perform a link test to check if the functions are present in libc; no need to have a declaration present. >> 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. > > Right, I'll make the change. Thanks ! > >> > 4) ctype_configure_char.cc >> > I've some troubles knowing what is supposed to be implemented on this file. >> > I don't really understand the part with setlocale which appears in many >> > os. When I'm adding it, some tests start failing, some start working... >> > Moreover, on Linux, if I understand correctly, there is some optimizations >> > based on classic_table(), _M_toupper and _M_tolower. Could you confirm >> > 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. > > I might have found the correct code since yesterday's mail. The problem seems > to come from _M_c_locale_ctype initialization. With locale support, it must be > _S_clone_c_locale(__cloc), without it, it must be the default locale which > ends up > being "C". I might push a newer patch this afternoon, with the correct code. Nice; I'll certainly give it a whirl! >> > Feel free to try in on other OS. But I've made modifications only for AIX >> > and >> > Linux, as I can test the other ones. >> >> 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. > > Right, because I don't know how to handle them (and I've forgotten to ask > for it...). > The first is for typedef __c_locale. It seems to be int* instead of locale_t. > Could you confirm that this is wanted and mandatory ? > The second is in about some functions in ctype_members.cc which are > defined in config/os/../ctype_inlines.h for FreeBSD and Dragonfly. Someone > has to confirm that it can be merged with the new code, or if this is > mandatory. That someone would most likely be Jonathan ;-) >> 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 = 15, as the generic clocale >> implementation uses. > > If I'm not mistaking, POSIX is only defining 11 bit for ctype. If we want > some optimizations we can have a define of bitmasksize or we can simply > fill the whole mask by setting bitmasksize=15 as in generic. > I don't know what's best. AFAIK the constants used in <ctype.h> (or <iso/ctype_iso.h> on Solaris) to describe the different character classes are just an implementation detail of the respective OS; certainly none of them are listed on the OpenGroup page for XPG7 <ctype.h>. So OSes are free to implement this any way they wish, and the generic variant (and Solaris) show that 11 bits are not enough on some. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University