Skip to content

Commit 009f05c

Browse files
authored
Make the build work with -Wformat=2 (#2088)
* Make the build work with -Wformat=2 The -Wformat=2 macro requires every format string either be a string literal, or a parameter that was tagged with the format attribute. The upshot is that it expects every printf function to be marked, or you get warnings like this: src/console_reporter.cc:104:23: error: format string is not a string literal [-Werror,-Wformat-nonliteral] 104 | out << FormatString(fmt, args); | ^~~ Marking such things is generally worthwhile since it turns on error-checking within the library, so fill in the missing ones. Tested with: bazelisk build --copt=-Werror --copt=-Wformat=2 :all * Add -Wformat=2 to catch regressions
1 parent 6a8dee9 commit 009f05c

File tree

6 files changed

+16
-10
lines changed

6 files changed

+16
-10
lines changed

BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ COPTS = [
1212
"-Wshadow",
1313
# "-Wshorten-64-to-32",
1414
"-Wfloat-equal",
15+
"-Wformat=2",
1516
"-fstrict-aliasing",
1617
## assert() are used a lot in tests upstream, which may be optimised out leading to
1718
## unused-variable warning.

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ else()
197197
add_cxx_compiler_flag(-Wfloat-equal)
198198
add_cxx_compiler_flag(-Wold-style-cast)
199199
add_cxx_compiler_flag(-Wconversion)
200+
add_cxx_compiler_flag(-Wformat=2)
200201
if(BENCHMARK_ENABLE_WERROR)
201202
add_cxx_compiler_flag(-Werror)
202203
endif()

src/colorprint.h

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#include <iostream>
66
#include <string>
77

8+
#include "internal_macros.h"
9+
810
namespace benchmark {
911
enum LogColor {
1012
COLOR_DEFAULT,
@@ -17,16 +19,6 @@ enum LogColor {
1719
COLOR_WHITE
1820
};
1921

20-
#if defined(__GNUC__) || defined(__clang__)
21-
#define PRINTF_FORMAT_STRING_FUNC(format_arg, first_idx) \
22-
__attribute__((format(printf, format_arg, first_idx)))
23-
#elif defined(__MINGW32__)
24-
#define PRINTF_FORMAT_STRING_FUNC(format_arg, first_idx) \
25-
__attribute__((format(__MINGW_PRINTF_FORMAT, format_arg, first_idx)))
26-
#else
27-
#define PRINTF_FORMAT_STRING_FUNC(format_arg, first_idx)
28-
#endif
29-
3022
PRINTF_FORMAT_STRING_FUNC(1, 0)
3123
std::string FormatString(const char* msg, va_list args);
3224
PRINTF_FORMAT_STRING_FUNC(1, 2) std::string FormatString(const char* msg, ...);

src/console_reporter.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ void ConsoleReporter::ReportRuns(const std::vector<Run>& reports) {
9797
}
9898
}
9999

100+
PRINTF_FORMAT_STRING_FUNC(3, 4)
100101
static void IgnoreColorPrint(std::ostream& out, LogColor /*unused*/,
101102
const char* fmt, ...) {
102103
va_list args;

src/internal_macros.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,16 @@
106106
#define BENCHMARK_MAYBE_UNUSED
107107
#endif
108108

109+
#if defined(__GNUC__) || defined(__clang__)
110+
#define PRINTF_FORMAT_STRING_FUNC(format_arg, first_idx) \
111+
__attribute__((format(printf, format_arg, first_idx)))
112+
#elif defined(__MINGW32__)
113+
#define PRINTF_FORMAT_STRING_FUNC(format_arg, first_idx) \
114+
__attribute__((format(__MINGW_PRINTF_FORMAT, format_arg, first_idx)))
115+
#else
116+
#define PRINTF_FORMAT_STRING_FUNC(format_arg, first_idx)
117+
#endif
118+
109119
// clang-format on
110120

111121
#endif // BENCHMARK_INTERNAL_MACROS_H_

src/string_util.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ std::string ToBinaryStringFullySpecified(double value, int precision,
112112
return mantissa + ExponentToPrefix(exponent, one_k == Counter::kIs1024);
113113
}
114114

115+
PRINTF_FORMAT_STRING_FUNC(1, 0)
115116
std::string StrFormatImp(const char* msg, va_list args) {
116117
// we might need a second shot at this, so pre-emptivly make a copy
117118
va_list args_cp;

0 commit comments

Comments
 (0)