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

[Vadim Zeitlin]

On Sun, 22 May 2022 20:33:24 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

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

 This is a perfectly understandable point of view, but OTOH MSVC CRT has
significantly changed during the switch to the "universal CRT" (ucrt) and
is now much more compliant than before (at the price of not being
compatible with the expectations of the code written in the nineties any
longer). So I think relying on snrpintf() to work correctly in it should be
a much safer bet than relying on all digits of expm1() to be exact.

GC> If there's some silly code we could add to defeat the "tidy"
GC> check, such as
GC> 
GC>   #if defined __GNUC__
GC> - #   if !defined _ISOC99_SOURCE
GC> + #   if defined _ISOC99_SOURCE
GC> + #     TELL_CLANG_TIDY_TO_OVERLOOK(_ISOC99_SOURCE)
GC> - #   if !defined _ISOC99_SOURCE
GC> + #   else  // !defined _ISOC99_SOURCE
GC>   #       define _ISOC99_SOURCE
GC>   #   endif // !defined _ISOC99_SOURCE
GC>   #endif // defined __GNUC__
GC> 
GC> then that might be a good solution. In any event I will at
GC> least add a comment citing this message.

 I'll give an even better solution below, but, as I wrote in my original
message about clang-tidy, the general solution is to add

        // NOLINTNEXTLINE(bugprone-reserved-identifier)

before _ISOC99_SOURCE definition. And I'd still like to know if you'd
accept adding such comments because there are definitely going to be other
places where they would be needed. For example, I'd like to add

        // NOLINTBEGIN(bugprone-macro-parentheses)

before LMI_ASSERT_WITH_MSG() macro definition and

        // NOLINTEND(bugprone-macro-parentheses)

after it because the expansion of this macro contains "message" which is
not in the parentheses, and can't be surrounded by them, as the intention
here is to allow passing any sequence of "<<"-separated expressions to the
macro. But this triggers the bugprone-macro-parentheses clang-tidy check
that I'd like to enable because it's a useful check, generally speaking.
And here I don't see any other workaround.


 But in the case of _ISOC99_SOURCE I realized that I did, in fact, have a
pretty simple workaround: it's enough to simply define it on clang-tidy
command line, i.e. add -D_ISOC99_SOURCE to it. This ensures that the line
(re)defining is overlooked because it's not actually taken into account.

 So this particular can be considered fixed, but:

1. Not critical at all, but I still think it would be better to let MinGW
   use the default stdio implementation (which is the UCRT one nowadays).

2. More importantly, I still need to know if we can use NOLINTxxx comments
   in lmi code or if we can't enable any checks that can't be fixed
   everywhere.

 Could you please let me know what do you think of (2)? Thanks in advance!
VZ

pgpBtpaQFrX1f.pgp
Description: PGP signature