Skip to content

Commit fd7b05a

Browse files
devbwwdevbww
authored
cleanup: enable clang-tidy for bigtable headers (#4111)
Note that some "performance-unnecessary-value-param" NOLINT directives are only due to an apparent clang-tidy bug, and are tagged as "TODO(#4112)" to make their later removal easy. Part of #3958.
1 parent a1deb43 commit fd7b05a

41 files changed

Lines changed: 186 additions & 134 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.clang-tidy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ Checks: >
5656
WarningsAsErrors: "*"
5757

5858
# TODO(#3920) - Enable clang-tidy checks in our headers.
59-
HeaderFilterRegex: "google/cloud/(bigquery|firestore|pubsub|spanner)/.*"
59+
HeaderFilterRegex: "google/cloud/(bigquery|bigtable|firestore|pubsub|spanner)/.*"
6060

6161
CheckOptions:
6262
- { key: readability-identifier-naming.NamespaceCase, value: lower_case }

‎google/cloud/bigtable/app_profile_config.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,17 @@ class AppProfileConfig {
4949
return *this;
5050
}
5151

52-
// NOLINT: accessors can (and should) be snake_case.
5352
google::bigtable::admin::v2::CreateAppProfileRequest const& as_proto()
5453
const& {
5554
return proto_;
5655
}
5756

58-
// NOLINT: accessors can (and should) be snake_case.
5957
google::bigtable::admin::v2::CreateAppProfileRequest&& as_proto() && {
6058
return std::move(proto_);
6159
}
6260

6361
private:
64-
AppProfileConfig() : proto_() {}
62+
AppProfileConfig() = default;
6563

6664
private:
6765
google::bigtable::admin::v2::CreateAppProfileRequest proto_;
@@ -70,7 +68,7 @@ class AppProfileConfig {
7068
/// Build a proto to update an Application Profile configuration.
7169
class AppProfileUpdateConfig {
7270
public:
73-
AppProfileUpdateConfig() : proto_() {}
71+
AppProfileUpdateConfig() = default;
7472

7573
AppProfileUpdateConfig& set_ignore_warnings(bool value) {
7674
proto_.set_ignore_warnings(value);
@@ -105,13 +103,11 @@ class AppProfileUpdateConfig {
105103
return *this;
106104
}
107105

108-
// NOLINT: accessors can (and should) be snake_case.
109106
google::bigtable::admin::v2::UpdateAppProfileRequest const& as_proto()
110107
const& {
111108
return proto_;
112109
}
113110

114-
// NOLINT: accessors can (and should) be snake_case.
115111
google::bigtable::admin::v2::UpdateAppProfileRequest&& as_proto() && {
116112
return std::move(proto_);
117113
}

‎google/cloud/bigtable/async_row_reader.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class AsyncRowReader : public std::enable_shared_from_this<
4646
AsyncRowReader<RowFunctor, FinishFunctor>> {
4747
public:
4848
/// Special value to be used as rows_limit indicating no limit.
49+
// NOLINTNEXTLINE(readability-identifier-naming)
4950
static std::int64_t constexpr NO_ROWS_LIMIT = 0;
5051
// Callbacks keep pointers to these objects.
5152
AsyncRowReader(AsyncRowReader&&) = delete;
@@ -63,11 +64,16 @@ class AsyncRowReader : public std::enable_shared_from_this<
6364
"RowFunctor should return a future<bool>.");
6465

6566
static std::shared_ptr<AsyncRowReader> Create(
67+
// NOLINTNEXTLINE(performance-unnecessary-value-param) TODO(#4112)
6668
CompletionQueue cq, std::shared_ptr<DataClient> client,
69+
// NOLINTNEXTLINE(performance-unnecessary-value-param) TODO(#4112)
6770
std::string app_profile_id, std::string table_name, RowFunctor on_row,
71+
// NOLINTNEXTLINE(performance-unnecessary-value-param) TODO(#4112)
6872
FinishFunctor on_finish, RowSet row_set, std::int64_t rows_limit,
73+
// NOLINTNEXTLINE(performance-unnecessary-value-param) TODO(#4112)
6974
Filter filter, std::unique_ptr<RPCRetryPolicy> rpc_retry_policy,
7075
std::unique_ptr<RPCBackoffPolicy> rpc_backoff_policy,
76+
// NOLINTNEXTLINE(performance-unnecessary-value-param) TODO(#4112)
7177
MetadataUpdatePolicy metadata_update_policy,
7278
std::unique_ptr<internal::ReadRowsParserFactory> parser_factory) {
7379
std::shared_ptr<AsyncRowReader> res(new AsyncRowReader(
@@ -233,7 +239,8 @@ class AsyncRowReader : public std::enable_shared_from_this<
233239
}
234240

235241
/// Called when lower layers provide us with a response chunk.
236-
future<bool> OnDataReceived(google::bigtable::v2::ReadRowsResponse response) {
242+
future<bool> OnDataReceived(
243+
google::bigtable::v2::ReadRowsResponse const& response) {
237244
// assert(!whole_op_finished_);
238245
// assert(!continue_reading_);
239246
// assert(status_.ok());
@@ -263,6 +270,7 @@ class AsyncRowReader : public std::enable_shared_from_this<
263270
}
264271

265272
/// Called when the whole stream finishes.
273+
// NOLINTNEXTLINE(performance-unnecessary-value-param)
266274
void OnStreamFinished(Status status) {
267275
// assert(!continue_reading_);
268276
if (status_.ok()) {

‎google/cloud/bigtable/cell.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,10 @@ class Cell {
104104

105105
/// Create a Cell and fill it with a 64-bit value encoded as big endian.
106106
template <typename KeyType, typename ColumnType>
107+
// NOLINTNEXTLINE(performance-unnecessary-value-param) TODO(#4112)
107108
Cell(KeyType&& row_key, std::string family_name,
108109
ColumnType&& column_qualifier, std::int64_t timestamp,
110+
// NOLINTNEXTLINE(performance-unnecessary-value-param) TODO(#4112)
109111
std::int64_t value, std::vector<std::string> labels)
110112
: Cell(std::forward<KeyType>(row_key), std::move(family_name),
111113
std::forward<ColumnType>(column_qualifier), timestamp,
@@ -114,6 +116,7 @@ class Cell {
114116

115117
/// Create a cell and fill it with data, but with empty labels.
116118
template <typename KeyType, typename ColumnType, typename ValueType>
119+
// NOLINTNEXTLINE(performance-unnecessary-value-param) TODO(#4112)
117120
Cell(KeyType&& row_key, std::string family_name,
118121
ColumnType&& column_qualifier, std::int64_t timestamp, ValueType&& value)
119122
: Cell(std::forward<KeyType>(row_key), std::move(family_name),

‎google/cloud/bigtable/cluster_config.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,20 @@ namespace google {
2222
namespace cloud {
2323
namespace bigtable {
2424
inline namespace BIGTABLE_CLIENT_NS {
25+
2526
/// Specify the initial configuration for a new cluster.
2627
class ClusterConfig {
2728
public:
2829
using StorageType = google::bigtable::admin::v2::StorageType;
30+
// NOLINTNEXTLINE(readability-identifier-naming)
2931
constexpr static StorageType STORAGE_TYPE_UNSPECIFIED =
3032
google::bigtable::admin::v2::STORAGE_TYPE_UNSPECIFIED;
33+
// NOLINTNEXTLINE(readability-identifier-naming)
3134
constexpr static StorageType SSD = google::bigtable::admin::v2::SSD;
35+
// NOLINTNEXTLINE(readability-identifier-naming)
3236
constexpr static StorageType HDD = google::bigtable::admin::v2::HDD;
3337

38+
// NOLINTNEXTLINE(google-explicit-constructor)
3439
ClusterConfig(google::bigtable::admin::v2::Cluster cluster)
3540
: proto_(std::move(cluster)) {}
3641

@@ -43,12 +48,10 @@ class ClusterConfig {
4348

4449
std::string const& GetName() { return proto_.name(); }
4550

46-
// NOLINT: accessors can (and should) be snake_case.
4751
google::bigtable::admin::v2::Cluster const& as_proto() const& {
4852
return proto_;
4953
}
5054

51-
// NOLINT: accessors can (and should) be snake_case.
5255
google::bigtable::admin::v2::Cluster&& as_proto() && {
5356
return std::move(proto_);
5457
}

‎google/cloud/bigtable/column_family.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,10 @@ class GcRule {
123123
std::is_convertible<GcRuleTypes, GcRule>...>::value,
124124
"The arguments to Union must be convertible to GcRule");
125125
GcRule tmp;
126-
auto& union_ = *tmp.gc_rule_.mutable_union_();
126+
auto& gc_rule_union = *tmp.gc_rule_.mutable_union_();
127127
std::initializer_list<GcRule> list{std::forward<GcRuleTypes>(gc_rules)...};
128128
for (GcRule const& rule : list) {
129-
*union_.add_rules() = rule.as_proto();
129+
*gc_rule_union.add_rules() = rule.as_proto();
130130
}
131131
return tmp;
132132
}
@@ -150,7 +150,7 @@ class GcRule {
150150
//@}
151151

152152
private:
153-
GcRule() {}
153+
GcRule() = default;
154154

155155
private:
156156
google::bigtable::admin::v2::GcRule gc_rule_;
@@ -217,7 +217,7 @@ class ColumnFamilyModification {
217217
//@}
218218

219219
private:
220-
ColumnFamilyModification() {}
220+
ColumnFamilyModification() = default;
221221

222222
private:
223223
::google::bigtable::admin::v2::ModifyColumnFamiliesRequest::Modification mod_;

‎google/cloud/bigtable/data_client.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ std::shared_ptr<DataClient> CreateDefaultDataClient(std::string project_id,
185185
* Compute the full path of the instance associated with the client, i.e.,
186186
* `projects/instances/<client->project_id()>/instances/<client->instance_id()>`
187187
*/
188-
inline std::string InstanceName(std::shared_ptr<DataClient> client) {
188+
inline std::string InstanceName(std::shared_ptr<DataClient> const& client) {
189189
return "projects/" + client->project_id() + "/instances/" +
190190
client->instance_id();
191191
}

‎google/cloud/bigtable/filters.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,9 @@ class Filter {
205205
template <typename Rep1, typename Period1, typename Rep2, typename Period2>
206206
static Filter TimestampRange(std::chrono::duration<Rep1, Period1> start,
207207
std::chrono::duration<Rep2, Period2> end) {
208-
using namespace std::chrono;
209-
return TimestampRangeMicros(duration_cast<microseconds>(start).count(),
210-
duration_cast<microseconds>(end).count());
208+
return TimestampRangeMicros(
209+
std::chrono::duration_cast<std::chrono::microseconds>(start).count(),
210+
std::chrono::duration_cast<std::chrono::microseconds>(end).count());
211211
}
212212

213213
/**
@@ -687,7 +687,7 @@ class Filter {
687687

688688
private:
689689
/// An empty filter, discards all data.
690-
Filter() : filter_() {}
690+
Filter() = default;
691691

692692
private:
693693
google::bigtable::v2::RowFilter filter_;

‎google/cloud/bigtable/idempotent_mutation_policy.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ namespace google {
2323
namespace cloud {
2424
namespace bigtable {
2525
inline namespace BIGTABLE_CLIENT_NS {
26+
2627
/**
2728
* Defines the interface to control which mutations are idempotent and therefore
2829
* can be re-tried.
@@ -53,7 +54,7 @@ std::unique_ptr<IdempotentMutationPolicy> DefaultIdempotentMutationPolicy();
5354
*/
5455
class SafeIdempotentMutationPolicy : public IdempotentMutationPolicy {
5556
public:
56-
SafeIdempotentMutationPolicy() {}
57+
SafeIdempotentMutationPolicy() = default;
5758

5859
std::unique_ptr<IdempotentMutationPolicy> clone() const override;
5960
bool is_idempotent(google::bigtable::v2::Mutation const&) override;
@@ -73,13 +74,14 @@ class SafeIdempotentMutationPolicy : public IdempotentMutationPolicy {
7374
*/
7475
class AlwaysRetryMutationPolicy : public IdempotentMutationPolicy {
7576
public:
76-
AlwaysRetryMutationPolicy() {}
77+
AlwaysRetryMutationPolicy() = default;
7778

7879
std::unique_ptr<IdempotentMutationPolicy> clone() const override;
7980
bool is_idempotent(google::bigtable::v2::Mutation const&) override;
8081
bool is_idempotent(
8182
google::bigtable::v2::CheckAndMutateRowRequest const&) override;
8283
};
84+
8385
} // namespace BIGTABLE_CLIENT_NS
8486
} // namespace bigtable
8587
} // namespace cloud

‎google/cloud/bigtable/instance_admin.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ class InstanceAdmin {
157157
* LimitedErrorCountRetryPolicy, LimitedTimeRetryPolicy.
158158
*/
159159
template <typename... Policies>
160+
// NOLINTNEXTLINE(performance-unnecessary-value-param) TODO(#4112)
160161
explicit InstanceAdmin(std::shared_ptr<InstanceAdminClient> client,
161162
Policies&&... policies)
162163
: InstanceAdmin(std::move(client)) {

0 commit comments

Comments
 (0)