lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] PATCH: fix math_functions_test build with clang


From: Vadim Zeitlin
Subject: Re: [lmi] PATCH: fix math_functions_test build with clang
Date: Sun, 22 May 2022 21:07:48 +0200

On Sun, 22 May 2022 15:39:14 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 5/21/22 13:40, Vadim Zeitlin wrote:
GC> [...]
GC> > [2]: https://github.com/let-me-illustrate/lmi/pull/210
GC> 
GC> I think the changes I just pushed take care of that entire PR.

 Yes, thank you!

GC> If "#pragma GCC' should be guarded with a test for LMI_GCC rather
GC> than for __GNUC__, then was it right to fix only a couple of cases,
GC> when many more exist?

 Perhaps not, but it doesn't create any immediate problems and I try to
minimize the number of changes needed for supporting clang.

GC> For example:
GC> 
GC>   $git grep -B3 'pragma GCC'
GC>   bourn_cast.hpp-#if defined __GNUC__
GC>   bourn_cast.hpp:#   pragma GCC diagnostic push
GC>   bourn_cast.hpp:#   pragma GCC diagnostic ignored "-Wsign-compare"
GC>   bourn_cast.hpp:#   pragma GCC diagnostic ignored "-Wsign-conversion"
GC> 
GC> Why isn't that a problem for clang, which AIUI defines __GNUC__ ?

 Because clang has the same warning options with the same meaning, so
disabling them works (and is also helpful) for it too. OTOH all versions of
clang, even the most recent ones, define __GNUC__ as 4, so the pragmas
guarded by the checks for a specific gcc version, such as the one for
-Wbool-compare, in the same file, are not used with clang -- which is
fortuitous because it would have resulted in a warning/error with it, as
clang doesn't have this warning.

 So right now everything works as intended, but if you decide to remove all
checks for gcc version (because we definitely don't support anything below
10 anyhow by now), it would break clang build and from this point of view
it might be better to always check for LMI_GCC. But this would (probably, I
didn't test it, but I'm almost sure it will) require adding pragmas for
clang, inside LMI_CLANG checks, too, and many of them would duplicate the
existing pragmas for gcc. So I'm not sure if you really want to do this.

 I could test this to make it more precise, i.e. check which of the pragmas
currently used with gcc are really needed (and not just harmless) for clang
too. Please let me know if you'd like me to do it.

 Thanks,
VZ

Attachment: pgpIoIhqcBf5W.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]