Hi Rainer > > 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 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. > 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. > 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. > 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. > > 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 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. > 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 This is normal, because I need a way to check for which locale model is being used. Thanks, Clément
