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

[Vadim Zeitlin]

On Sun, 22 May 2022 22:58:53 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 5/22/22 22:20, Vadim Zeitlin wrote:
GC> > On Sun, 22 May 2022 20:33:24 +0000 Greg Chicares 
<gchicares@sbcglobal.net> wrote:
[...]
GC> > GC> If there's some silly code we could add to defeat the "tidy"
GC> > GC> check
GC> [...]
GC> >  I'll give an even better solution below, but, as I wrote in my original
GC> > message about clang-tidy, the general solution is to add
GC> > 
GC> >   // NOLINTNEXTLINE(bugprone-reserved-identifier)
GC> > 
GC> > before _ISOC99_SOURCE definition.
GC> 
GC> Perfect.
GC> 
GC> > And I'd still like to know if you'd
GC> > accept adding such comments because there are definitely going to be other
GC> > places where they would be needed. For example, I'd like to add [...]
GC> 
GC> Perfect, too.

 I didn't even hope for such perfect (sic) answers, but it's definitely the
simplest solution from my point of view, so I'm very glad to get them and
will use the NOLINT comments (in moderation) then.

GC> >  But in the case of _ISOC99_SOURCE I realized that I did, in fact, have a
GC> > pretty simple workaround: it's enough to simply define it on clang-tidy
GC> > command line, i.e. add -D_ISOC99_SOURCE to it.
GC> 
GC> I'd rather use the "NOLINT" thing everywhere:
GC>  - one class of problems --> one technique to forestall them all
GC>  - it's easy to forget to use a command-line parameter

 In principle, clang-tidy is supposed to be executed with the same
compilation flags that are used for building. I'm not sure how am I going
to handle this yet, as there are a couple of possibilities: in addition to
the straightforward one of integrating it into the makefiles, it's also
possible to use BEAR (https://github.com/rizsotto/Bear) to build the so
called compilation database, that can be used by all clang tools,
automatically during the normal build. And if we do this, it's definitely
going to be much simpler _not_ to add -D_ISOC99_SOURCE to all the commands
in this compilation database, so it's another argument in favour of using
NOLINT.

GC> I'd be glad to apply a patch that uses "NOLINT" wherever
GC> it's needed, as long as it's needed only in a reasonable
GC> number of cases, of course.

 This depends on the checks that we want to enable. If it's just
bugprone-reserved-identifier, it's definitely not going to be a problem.
But other potentially useful checks could require adding more exceptions. I
don't know which checks are going to be useful, there are a lot of them
(see the list at https://clang.llvm.org/extra/clang-tidy/checks/list.html)
and while many of them are clearly irrelevant (anything specific to
Android, Fuchsia, LLVM etc, for example), some could be useful (this
includes several C++ core guidelines), but do result in a lot of warnings
right now.

 I'm still undecided about what would be best, but I guess it's better to
err on the side of getting fewer false positives, even at the cost of
missing some potentially helpful warnings too, isn't it?

VZ

pgpJCp8i0WX4U.pgp
Description: PGP signature