Skip to content

Commit ab86707

Browse files
dominc8dominc8
authored
clang-tidy: readability-redundant and performance (google#1298)
* clang-tidy: readability-redundant-* * clang-tidy: performance-*
1 parent fd258bb commit ab86707

13 files changed

+34
-37
lines changed

‎.clang-tidy

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
2-
Checks: 'clang-analyzer-*'
3-
WarningsAsErrors: 'clang-analyzer-*'
2+
Checks: 'clang-analyzer-*,readability-redundant-*,performance-*'
3+
WarningsAsErrors: 'clang-analyzer-*,readability-redundant-*,performance-*'
44
HeaderFilterRegex: '.*'
55
AnalyzeTemporaryDtors: false
66
FormatStyle: none

‎include/benchmark/benchmark.h

+9-13
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,7 @@ class State {
774774
bool finished_;
775775
bool error_occurred_;
776776

777-
private: // items we don't need on the first cache line
777+
// items we don't need on the first cache line
778778
std::vector<int64_t> range_;
779779

780780
int64_t complexity_n_;
@@ -1054,7 +1054,8 @@ class Benchmark {
10541054
Benchmark* Complexity(BigOFunc* complexity);
10551055

10561056
// Add this statistics to be computed over all the values of benchmark run
1057-
Benchmark* ComputeStatistics(std::string name, StatisticsFunc* statistics,
1057+
Benchmark* ComputeStatistics(const std::string& name,
1058+
StatisticsFunc* statistics,
10581059
StatisticUnit unit = kTime);
10591060

10601061
// Support for running multiple copies of the same benchmark concurrently
@@ -1169,8 +1170,7 @@ class LambdaBenchmark : public Benchmark {
11691170

11701171
LambdaBenchmark(LambdaBenchmark const&) = delete;
11711172

1172-
private:
1173-
template <class Lam>
1173+
template <class Lam> // NOLINTNEXTLINE(readability-redundant-declaration)
11741174
friend Benchmark* ::benchmark::RegisterBenchmark(const char*, Lam&&);
11751175

11761176
Lambda lambda_;
@@ -1338,7 +1338,7 @@ class Fixture : public internal::Benchmark {
13381338
#define BENCHMARK_PRIVATE_DECLARE_F(BaseClass, Method) \
13391339
class BaseClass##_##Method##_Benchmark : public BaseClass { \
13401340
public: \
1341-
BaseClass##_##Method##_Benchmark() : BaseClass() { \
1341+
BaseClass##_##Method##_Benchmark() { \
13421342
this->SetName(#BaseClass "/" #Method); \
13431343
} \
13441344
\
@@ -1349,7 +1349,7 @@ class Fixture : public internal::Benchmark {
13491349
#define BENCHMARK_TEMPLATE1_PRIVATE_DECLARE_F(BaseClass, Method, a) \
13501350
class BaseClass##_##Method##_Benchmark : public BaseClass<a> { \
13511351
public: \
1352-
BaseClass##_##Method##_Benchmark() : BaseClass<a>() { \
1352+
BaseClass##_##Method##_Benchmark() { \
13531353
this->SetName(#BaseClass "<" #a ">/" #Method); \
13541354
} \
13551355
\
@@ -1360,7 +1360,7 @@ class Fixture : public internal::Benchmark {
13601360
#define BENCHMARK_TEMPLATE2_PRIVATE_DECLARE_F(BaseClass, Method, a, b) \
13611361
class BaseClass##_##Method##_Benchmark : public BaseClass<a, b> { \
13621362
public: \
1363-
BaseClass##_##Method##_Benchmark() : BaseClass<a, b>() { \
1363+
BaseClass##_##Method##_Benchmark() { \
13641364
this->SetName(#BaseClass "<" #a "," #b ">/" #Method); \
13651365
} \
13661366
\
@@ -1372,7 +1372,7 @@ class Fixture : public internal::Benchmark {
13721372
#define BENCHMARK_TEMPLATE_PRIVATE_DECLARE_F(BaseClass, Method, ...) \
13731373
class BaseClass##_##Method##_Benchmark : public BaseClass<__VA_ARGS__> { \
13741374
public: \
1375-
BaseClass##_##Method##_Benchmark() : BaseClass<__VA_ARGS__>() { \
1375+
BaseClass##_##Method##_Benchmark() { \
13761376
this->SetName(#BaseClass "<" #__VA_ARGS__ ">/" #Method); \
13771377
} \
13781378
\
@@ -1539,7 +1539,6 @@ class BenchmarkReporter {
15391539
complexity_n(0),
15401540
report_big_o(false),
15411541
report_rms(false),
1542-
counters(),
15431542
memory_result(NULL),
15441543
allocs_per_iter(0.0) {}
15451544

@@ -1676,10 +1675,7 @@ class ConsoleReporter : public BenchmarkReporter {
16761675
OO_Defaults = OO_ColorTabular
16771676
};
16781677
explicit ConsoleReporter(OutputOptions opts_ = OO_Defaults)
1679-
: output_options_(opts_),
1680-
name_field_width_(0),
1681-
prev_counters_(),
1682-
printed_header_(false) {}
1678+
: output_options_(opts_), name_field_width_(0), printed_header_(false) {}
16831679

16841680
virtual bool ReportContext(const Context& context) BENCHMARK_OVERRIDE;
16851681
virtual void ReportRuns(const std::vector<Run>& reports) BENCHMARK_OVERRIDE;

‎src/benchmark.cc

+2-3
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ State::State(IterationCount max_iters, const std::vector<int64_t>& ranges,
145145
error_occurred_(false),
146146
range_(ranges),
147147
complexity_n_(0),
148-
counters(),
149148
thread_index_(thread_i),
150149
threads_(n_threads),
151150
timer_(timer),
@@ -434,7 +433,7 @@ size_t RunSpecifiedBenchmarks() {
434433
}
435434

436435
size_t RunSpecifiedBenchmarks(std::string spec) {
437-
return RunSpecifiedBenchmarks(nullptr, nullptr, spec);
436+
return RunSpecifiedBenchmarks(nullptr, nullptr, std::move(spec));
438437
}
439438

440439
size_t RunSpecifiedBenchmarks(BenchmarkReporter* display_reporter) {
@@ -444,7 +443,7 @@ size_t RunSpecifiedBenchmarks(BenchmarkReporter* display_reporter) {
444443

445444
size_t RunSpecifiedBenchmarks(BenchmarkReporter* display_reporter,
446445
std::string spec) {
447-
return RunSpecifiedBenchmarks(display_reporter, nullptr, spec);
446+
return RunSpecifiedBenchmarks(display_reporter, nullptr, std::move(spec));
448447
}
449448

450449
size_t RunSpecifiedBenchmarks(BenchmarkReporter* display_reporter,

‎src/benchmark_register.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ Benchmark* Benchmark::Complexity(BigOFunc* complexity) {
413413
return this;
414414
}
415415

416-
Benchmark* Benchmark::ComputeStatistics(std::string name,
416+
Benchmark* Benchmark::ComputeStatistics(const std::string& name,
417417
StatisticsFunc* statistics,
418418
StatisticUnit unit) {
419419
statistics_.emplace_back(name, statistics, unit);

‎src/thread_manager.h

-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ class ThreadManager {
3636
[this]() { return alive_threads_ == 0; });
3737
}
3838

39-
public:
4039
struct Result {
4140
IterationCount iterations = 0;
4241
double real_time_used = 0;

‎test/args_product_test.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,15 @@ class ArgsProductFixture : public ::benchmark::Fixture {
3737
virtual ~ArgsProductFixture() {
3838
if (actualValues != expectedValues) {
3939
std::cout << "EXPECTED\n";
40-
for (auto v : expectedValues) {
40+
for (const auto& v : expectedValues) {
4141
std::cout << "{";
4242
for (int64_t iv : v) {
4343
std::cout << iv << ", ";
4444
}
4545
std::cout << "}\n";
4646
}
4747
std::cout << "ACTUAL\n";
48-
for (auto v : actualValues) {
48+
for (const auto& v : actualValues) {
4949
std::cout << "{";
5050
for (int64_t iv : v) {
5151
std::cout << iv << ", ";

‎test/benchmark_random_interleaving_gtest.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ TEST_F(BenchmarkTest, Match1WithRandomInterleaving) {
111111
std::vector<std::string> interleaving;
112112
interleaving.push_back(queue->Get());
113113
interleaving.push_back(queue->Get());
114-
element_count[interleaving[0].c_str()]++;
115-
element_count[interleaving[1].c_str()]++;
114+
element_count[interleaving[0]]++;
115+
element_count[interleaving[1]]++;
116116
interleaving_count[StrFormat("%s,%s", interleaving[0].c_str(),
117117
interleaving[1].c_str())]++;
118118
}

‎test/complexity_test.cc

+4-3
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ namespace {
1313
#define ADD_COMPLEXITY_CASES(...) \
1414
int CONCAT(dummy, __LINE__) = AddComplexityTest(__VA_ARGS__)
1515

16-
int AddComplexityTest(std::string test_name, std::string big_o_test_name,
17-
std::string rms_test_name, std::string big_o,
18-
int family_index) {
16+
int AddComplexityTest(const std::string &test_name,
17+
const std::string &big_o_test_name,
18+
const std::string &rms_test_name,
19+
const std::string &big_o, int family_index) {
1920
SetSubstitutions({{"%name", test_name},
2021
{"%bigo_name", big_o_test_name},
2122
{"%rms_name", rms_test_name},

‎test/multiple_ranges_test.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ class MultipleRangesFixture : public ::benchmark::Fixture {
4242
virtual ~MultipleRangesFixture() {
4343
if (actualValues != expectedValues) {
4444
std::cout << "EXPECTED\n";
45-
for (auto v : expectedValues) {
45+
for (const auto& v : expectedValues) {
4646
std::cout << "{";
4747
for (int64_t iv : v) {
4848
std::cout << iv << ", ";
4949
}
5050
std::cout << "}\n";
5151
}
5252
std::cout << "ACTUAL\n";
53-
for (auto v : actualValues) {
53+
for (const auto& v : actualValues) {
5454
std::cout << "{";
5555
for (int64_t iv : v) {
5656
std::cout << iv << ", ";

‎test/output_test.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ std::string GetFileReporterOutput(int argc, char* argv[]);
8585
struct Results;
8686
typedef std::function<void(Results const&)> ResultsCheckFn;
8787

88-
size_t AddChecker(const char* bm_name_pattern, ResultsCheckFn fn);
88+
size_t AddChecker(const char* bm_name_pattern, const ResultsCheckFn& fn);
8989

9090
// Class holding the results of a benchmark.
9191
// It is passed in calls to checker functions.

‎test/output_test_helper.cc

+6-5
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ void CheckCases(TestCaseList const& checks, std::stringstream& output) {
141141
class TestReporter : public benchmark::BenchmarkReporter {
142142
public:
143143
TestReporter(std::vector<benchmark::BenchmarkReporter*> reps)
144-
: reporters_(reps) {}
144+
: reporters_(std::move(reps)) {}
145145

146146
virtual bool ReportContext(const Context& context) BENCHMARK_OVERRIDE {
147147
bool last_ret = false;
@@ -183,15 +183,15 @@ class ResultsChecker {
183183
public:
184184
struct PatternAndFn : public TestCase { // reusing TestCase for its regexes
185185
PatternAndFn(const std::string& rx, ResultsCheckFn fn_)
186-
: TestCase(rx), fn(fn_) {}
186+
: TestCase(rx), fn(std::move(fn_)) {}
187187
ResultsCheckFn fn;
188188
};
189189

190190
std::vector<PatternAndFn> check_patterns;
191191
std::vector<Results> results;
192192
std::vector<std::string> field_names;
193193

194-
void Add(const std::string& entry_pattern, ResultsCheckFn fn);
194+
void Add(const std::string& entry_pattern, const ResultsCheckFn& fn);
195195

196196
void CheckResults(std::stringstream& output);
197197

@@ -210,7 +210,8 @@ ResultsChecker& GetResultsChecker() {
210210
}
211211

212212
// add a results checker for a benchmark
213-
void ResultsChecker::Add(const std::string& entry_pattern, ResultsCheckFn fn) {
213+
void ResultsChecker::Add(const std::string& entry_pattern,
214+
const ResultsCheckFn& fn) {
214215
check_patterns.emplace_back(entry_pattern, fn);
215216
}
216217

@@ -299,7 +300,7 @@ std::vector<std::string> ResultsChecker::SplitCsv_(const std::string& line) {
299300

300301
} // end namespace internal
301302

302-
size_t AddChecker(const char* bm_name, ResultsCheckFn fn) {
303+
size_t AddChecker(const char* bm_name, const ResultsCheckFn& fn) {
303304
auto& rc = internal::GetResultsChecker();
304305
rc.Add(bm_name, fn);
305306
return rc.results.size();

‎test/register_benchmark_test.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ struct TestCase {
4545
std::vector<TestCase> ExpectedResults;
4646

4747
int AddCases(std::initializer_list<TestCase> const& v) {
48-
for (auto N : v) {
48+
for (const auto& N : v) {
4949
ExpectedResults.push_back(N);
5050
}
5151
return 0;

‎test/skip_with_error_test.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,13 @@ ADD_CASES("BM_error_during_running", {{"/1/threads:1", true, "error message"},
119119

120120
void BM_error_during_running_ranged_for(benchmark::State& state) {
121121
assert(state.max_iterations > 3 && "test requires at least a few iterations");
122-
[[maybe_unused]] bool first_iter = true;
122+
bool first_iter = true;
123123
// NOTE: Users should not write the for loop explicitly.
124124
for (auto It = state.begin(), End = state.end(); It != End; ++It) {
125125
if (state.range(0) == 1) {
126126
assert(first_iter);
127127
first_iter = false;
128+
(void)first_iter;
128129
state.SkipWithError("error message");
129130
// Test the unfortunate but documented behavior that the ranged-for loop
130131
// doesn't automatically terminate when SkipWithError is set.

0 commit comments

Comments
 (0)