Alternative and more robust way of managing and working around #2708

[phelter]

Original issue: #2708

When integrating in with various linters (clang-tidy-18) and building with the GNU compilers using cmake at the same time, the current definition of stringop-overflow doesn't work because clang attempts to check the -Wno-stringop-overflow compiler switch and is currently unaware of that.

Compiler GNU: g++-14
And Cmake enabling of: clang-tidy-18 at the same time.

To mitigate this - I've had to do the following:

fmt_lib.h

#pragma once

#if !defined(__clang__) && defined(__GNUC__) && (__GNUC__ >= 7)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wstringop-overflow"
#endif // !defined(__clang__) && defined(__GNUC__) && (__GNUC__ >= 7)

#include <fmt/chrono.h>
#include <fmt/format.h>

#if !defined(__clang__) && defined(__GNUC__) && (__GNUC__ >= 7)
#pragma GCC diagnostic pop
#endif // !defined(__clang__) && defined(__GNUC__) && (__GNUC__ >= 7)

Instead of adding a compiler directive in the compile options, would it be possible to just add the above to the header-only library at the beginning and end of any respective fmt/*.h file that has this stringop-overflow issue?

Rationale:

• Since this is a header only library - if using the compiler option -Wno-stringop-overflow, it will be enabled for all consumers of the fmt::fmt or fmt::fmt-header-only INTERFACE CMake target. This is not ideal since I'd like the stringop-overflow to take effect and be checked in other areas of the code.

Much appreciated.

[vitaut]

clang attempts to check the -Wno-stringop-overflow compiler switch and is currently unaware of that

-Wno-stringop-overflow is only used in tests so I don't think it's a problem.

I don't think we should clutter the code to workaround a bug in gcc but you can use methods described in #2708 to suppress it if needed.

[phelter]

-Wno-stringop-overflow is only used in tests so I don't think it's a problem.

I also use as part of an include in other items and then test those other items. The workaround you specified in #2708 does NOT fix the specific issue that is provided in the description (another use case of tooling).

So it is a half fix for some of the tooling use cases where GCC is used on it's own.

@vitaut I agree the bug/issue is with the versions of clang and gnu compiler you use and how you use those in conjunction with eachother.

If you don't wish the user experience of all use cases then why bother putting the workaround inside the CMakeLists.txt file in the first place?

A lot of other open-source libraries provide #pragma disablement in their definitions so I don't believe your assessment of this fix being clutter is correct. It is documentation of what warnings/errors your code is willing to bypass (whether it is in the CMakeLists.txt or in code headers). A good example is : googletest pragmas It is required to be consumed by many different compilers in many different situations. They place the pragmas in the code to ensure proper operation across all of those various use cases.

The problem with putting it in the CMakeLists.txt as you specified is for particular use cases of tools that workaround (Using SYSTEM headers) does not work and therefore causes issue with consumption for all who use this library. Adding to every single target that consumes fmt/format.h as an INTERFACE or header only library means that all of those targets now need to also add the -Wno-stringop-overflow which means the fmtlib is dictating what compiler settings must be set on external target code outside the library rather than identifying the issue and stating that this library has assessed this issue and deemed it a false-positive.

If there is no willingness to move forward with the fix described above, I will continue to wrap all of the header inclusion of all fmt/*.h functions within my own layer of headers to work-around this issue. But I strongly suggest you provide a more openness of fixing both clang-tidy and compiler issues in the code itself.

[vitaut]

If you don't wish the user experience of all use cases then why bother putting the workaround inside the CMakeLists.txt file in the first place?

I am OK with removing that workaround.

Not completely sure why false positives in a specific version of GCC cause problem with your tooling. In any case it seems like a problem with the tooling and I think providing a wrapper header is a reasonable solution.

[vitaut]

The false positive reported in #2708 has been worked around in 7f157dc.

[vitaut]

The warning suppression in tests has been removed too, thanks for bringing this up.