Hi On Fri, Sep 8, 2023 at 10:28 AM Mike Fulton <mikefultonperso...@gmail.com> wrote:
> > >> >> On Thu, Sep 7, 2023 at 6:09 PM Eric Blake <ebl...@redhat.com> wrote: >> >>> On Wed, Sep 06, 2023 at 03:08:33PM -0700, Mike Fulton wrote: >>> > Hello, >>> > >>> > The m4 dev line ( git://git.savannah.gnu.org/m4.git ) >>> > has the following code: >>> > >>> > ``` >>> > #if 4 < __GNUC__ + (6 <= __GNUC_MINOR__) >>> > # pragma GCC diagnostic push >>> > # pragma GCC diagnostic ignored "-Wformat-nonliteral" >>> > #endif >>> > ``` >>> >>> That expression appears to be designed to reject gcc 3.5 and older, >>> while accepting 3.6 and newer. >>> >> Right - that's my understanding. >> >>> >>> I also wonder if modern gnulib provides a better way to test things >>> than just open-coding it (as Bruno has done a lot of work testing >>> features of newer compilers). This may be one case where ignoring the >>> diagnostic for older builds is desirable - there comes a point where >>> you can't please both old and new compilers simultaneously, but the >>> target that should be warning-free is the newer compiler. >>> >> Great question - this seems to be something added before clang and I >> wonder if there is a better way to manage diagnostics that are meant >> to be 'sort of common?' between GCC and clang. I've just started >> using clang on z/OS so I'm not sure how this mapping is done for >> GCC compatibility. >> >>> >>> That is, maybe it's time to just rewrite the check to something as >>> simple as #if 8 <= __GNUC__ (that is, only add the pragmas on a much >>> newer, albeit arbitrary, point in time)? >>> >>> Conversely, I see that m4 uses gnulib's lib/c-stack.c, which also has: >>> >>> /* Pacify GCC 9.3.1, which otherwise would complain about segv_handler. >>> */ >>> # if 4 < __GNUC__ + (6 <= __GNUC_MINOR__) >>> # pragma GCC diagnostic ignored "-Wsuggest-attribute=pure" >>> # endif >>> >>> so it's not like m4 is doing anything radically different from gnulib >>> when it comes to pragmas for ignoring certain diagnostics; but thd >>> difference may be which diagnostic is being ignored. >>> >>> > >>> > This produces an error on the clang compiler I am using on z/OS, which >>> is >>> > at 4.2 with the xasprintf code that follows, e.g. >>> > >>> > ``` >>> > str = xasprintf (fstart, width, ARG_INT(argc, argv)); >>> > ``` >>> > >>> > If I change the check to: >>> > >>> > ``` >>> > #if 4 < __GNUC__ + (2 <= __GNUC_MINOR__) >>> > # pragma GCC diagnostic push >>> > # pragma GCC diagnostic ignored "-Wformat-nonliteral" >>> > #endif >>> > ``` >>> > >>> > all is good - it compiles clean. The question is whether the check for >>> 6 is >>> > too high or if the clang compiler on z/OS has a bug (or something >>> else)? >>> >>> What does clang claim to be? That is, what do you get for >>> print '__GNUC__ __GNUC_MINOR__' | clang -E - | tail -n1 >>> >> On both z/OS as well as my Mac I see: >> 4 2 >> >>> >>> and what is the actual compiler error you got when the build failed on >>> the original source? >>> >> CC format.o >> format.c:360:28: error: format string is not a string literal >> [-Werror,-Wformat-nonliteral] >> str = xasprintf (fstart, width, ARG_INT(argc, argv)); >> >> ^~~~~~ >> >>> >>> > I haven't been able to determine what level of gcc provides this >>> diagnostic. >>> >> > What if we just acknowledged the clang compiler and changed the check to: > > #if 4 < __GNUC__ + (6 <= __GNUC_MINOR__) || defined(__clang__) > > The current clang compiler (not an old one) states that it's __GNU__C is 4 and it's __GNUC_MINOR__ is 2 (for both the Mac and z/OS - and I assume other platforms too - but I don't have access to other systems). So - I would like to propose that we extend the macro as I described above to add a check for the clang compiler in addition to the GNUC compiler to enable the #pragmas which will eliminate warnings (or in my case errors because I flag warnings as errors with my build). The fix is: if 4 < __GNUC__ + (6 <= __GNUC_MINOR__) || defined(__clang__) I would be happy to either provide a patch or have someone with the right authority, provide the patch. ? > >> > >>> > thanks, mike >>> >>> -- >>> Eric Blake, Principal Software Engineer >>> Red Hat, Inc. >>> Virtualization: qemu.org | libguestfs.org >>> >>>
