Re: [lmi] xanadu/remove-ISOC99_SOURCE-def

[Greg Chicares]

On 5/22/22 18:56, Vadim Zeitlin wrote:
> On Sun, 22 May 2022 15:45:30 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
> 
> GC> I haven't applied this:
> GC> 
> GC> -// This selects a correct snprintf() for MinGW-w64.
> GC> -
> GC> -#if defined __GNUC__
> GC> -#   if !defined _ISOC99_SOURCE
> GC> -#       define _ISOC99_SOURCE
> GC> -#   endif // !defined _ISOC99_SOURCE
> GC> -#endif // defined __GNUC__
> GC> 
> GC> because, for the historical libmingwex at least,
> GC> some macro such as _ISOC99_SOURCE was required to
> GC> override a defective msvcrt snprintf(). IIRC, some
> GC> other macros served the same purpose; this one
> GC> seemed the least objectionable.
> GC> 
> GC> Do you know for sure that the MinGW-w64 project
> GC> has made that replacement unconditional?
> 
>  No, they haven't, there is still a complicated conditional checking for,
> among other things, _ISOC99_SOURCE, before defining __USE_MINGW_ANSI_STDIO
> in _mingw.h (located in /usr/x86_64-w64-mingw32/include on Debian systems).

Okay, then the original morass of conditionals persists.

>  However this condition also contains the following subexpression:
> 
>      || (defined (__cplusplus) && __cplusplus >= 201103L && 
> __MSVCRT_VERSION__ < 0xE00) \
> 
> which seems to indicate that it's not needed to enable MinGW own
> implementation when using MSVC CRT >= 1400 (which will always be the case
> nowadays), which has a (more) standard compliant snprintf() of its own.
> 
>  I also hoped that if there were any problems with using that snprintf(),
> they would be uncovered by the unit tests, notably snprintf_test.cpp, but
> it passes without problems.

The expm1() and log1p work I've recently done elsewhere suggests
to me that if an msvcrt function is known to be defective (whether
in the latest version or in some previous version that could still
be circulating in the wild), and we have a replacement that
demonstrably lacks those known defects, then it's better to favor
the replacement, forever.

>  So I think it should be safe to remove it. BTW, if we you don't want to do
> it, I think it would be better to predefine __USE_MINGW_ANSI_STDIO itself
> (which wouldn't help the clang-tidy check, of course, but this is unrelated
> to it) rather than doing it indirectly via _ISOC99_SOURCE.

I had thought of doing exactly that, years ago, because
__USE_MINGW_ANSI_STDIO is descriptive and specific. I didn't do
that, though, because the conditionals seemed too tangled, and
stomping on them in that way seemed excessively brutal; for the
same reason, I hesitate to do that now.

If there's some silly code we could add to defeat the "tidy"
check, such as

  #if defined __GNUC__
- #   if !defined _ISOC99_SOURCE
+ #   if defined _ISOC99_SOURCE
+ #     TELL_CLANG_TIDY_TO_OVERLOOK(_ISOC99_SOURCE)
- #   if !defined _ISOC99_SOURCE
+ #   else  // !defined _ISOC99_SOURCE
  #       define _ISOC99_SOURCE
  #   endif // !defined _ISOC99_SOURCE
  #endif // defined __GNUC__

then that might be a good solution. In any event I will at
least add a comment citing this message.