tidy: enable some identifier naming rules This adds some basic identifier naming rules to our tidy config and fixes some of the aberrations from our style guide.
There are many more such options we can add over time, but we're more inconsistent in certain areas (eg FOO_BAR vs kFooBar for constants) and I wanted to start with the rules that we apply most consistently. Change-Id: I9749c4bef61ca780c3543a4894d289ef9a448ea9 Reviewed-on: http://gerrit.cloudera.org:8080/7157 Reviewed-by: Adar Dembo <[email protected]> Reviewed-by: Dan Burkert <[email protected]> Reviewed-by: Hao Hao <[email protected]> Tested-by: Kudu Jenkins Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/850ebef6 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/850ebef6 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/850ebef6 Branch: refs/heads/master Commit: 850ebef628650bbce193376343b8c468facf294b Parents: 636e2a9 Author: Todd Lipcon <[email protected]> Authored: Mon Jun 12 12:24:20 2017 -0700 Committer: Todd Lipcon <[email protected]> Committed: Mon Jun 12 23:12:07 2017 +0000 ---------------------------------------------------------------------- src/kudu/.clang-tidy | 13 +++++++++ src/kudu/benchmarks/tpch/tpch1.cc | 6 ++--- src/kudu/client/client-test.cc | 6 ++--- src/kudu/common/scan_spec.cc | 12 ++++----- src/kudu/experiments/rwlock-perf.cc | 28 ++++++++++---------- src/kudu/gutil/hash/hash.cc | 2 +- src/kudu/gutil/spinlock.cc | 6 ++--- .../full_stack-insert-scan-test.cc | 6 ++--- src/kudu/tools/ksck_remote.cc | 6 ++--- src/kudu/util/memcmpable_varint.cc | 20 +++++++------- src/kudu/util/memory/memory.cc | 12 ++++----- src/kudu/util/net/socket.cc | 4 +-- src/kudu/util/striped64.cc | 6 ++--- 13 files changed, 70 insertions(+), 57 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/.clang-tidy ---------------------------------------------------------------------- diff --git a/src/kudu/.clang-tidy b/src/kudu/.clang-tidy index d8a072d..de0a477 100644 --- a/src/kudu/.clang-tidy +++ b/src/kudu/.clang-tidy @@ -16,3 +16,16 @@ # under the License. Checks: 'clang-diagnostic-*,clang-analyzer-*,-clang-analyzer-alpha*,-*,readability-*,-readability-braces-around-statements,llvm-include-order,misc-*,-modernize-*,performance-*,-readability-implicit-bool-cast,google-*,-google-readability-braces-around-statements,-readability-redundant-string-init' HeaderFilterRegex: '.*,-*.pb.h' +CheckOptions: + - key: readability-identifier-naming.ClassCase + value: CamelCase + - key: readability-identifier-naming.EnumCase + value: CamelCase + - key: readability-identifier-naming.PrivateMemberSuffix + value: '_' + - key: readability-identifier-naming.ParameterCase + value: lower_case + - key: readability-identifier-naming.NamespaceCase + value: lower_case + - key: readability-identifier-naming.InlineNamespaceCase + value: lower_case http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/benchmarks/tpch/tpch1.cc ---------------------------------------------------------------------- diff --git a/src/kudu/benchmarks/tpch/tpch1.cc b/src/kudu/benchmarks/tpch/tpch1.cc index da36872..7cc95eb 100644 --- a/src/kudu/benchmarks/tpch/tpch1.cc +++ b/src/kudu/benchmarks/tpch/tpch1.cc @@ -129,7 +129,7 @@ struct SliceMapKey { } }; -struct hash { +struct Hash { size_t operator()(const SliceMapKey &key) const { return util_hash::CityHash64( reinterpret_cast<const char *>(key.slice.data()), key.slice.size()); @@ -154,8 +154,8 @@ void WarmupScanCache(RpcLineItemDAO* dao) { } void Tpch1(RpcLineItemDAO *dao) { - typedef unordered_map<SliceMapKey, Result*, hash> slice_map; - typedef unordered_map<SliceMapKey, slice_map*, hash> slice_map_map; + typedef unordered_map<SliceMapKey, Result*, Hash> slice_map; + typedef unordered_map<SliceMapKey, slice_map*, Hash> slice_map_map; gscoped_ptr<RpcLineItemDAO::Scanner> scanner; dao->OpenTpch1Scanner(&scanner); http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/client/client-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc index 244b80c..a7c5070 100644 --- a/src/kudu/client/client-test.cc +++ b/src/kudu/client/client-test.cc @@ -3911,16 +3911,16 @@ TEST_F(ClientTest, TestMasterLookupPermits) { namespace { class DLSCallback : public KuduStatusCallback { public: - explicit DLSCallback(Atomic32* i) : i(i) { + explicit DLSCallback(Atomic32* i) : i_(i) { } virtual void Run(const Status& s) OVERRIDE { CHECK_OK(s); - NoBarrier_AtomicIncrement(i, 1); + NoBarrier_AtomicIncrement(i_, 1); delete this; } private: - Atomic32* const i; + Atomic32* const i_; }; // Returns col1 value of first row. http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/common/scan_spec.cc ---------------------------------------------------------------------- diff --git a/src/kudu/common/scan_spec.cc b/src/kudu/common/scan_spec.cc index 9251972..98a99e4 100644 --- a/src/kudu/common/scan_spec.cc +++ b/src/kudu/common/scan_spec.cc @@ -81,16 +81,16 @@ void ScanSpec::SetExclusiveUpperBoundKey(const EncodedKey* key) { } } -void ScanSpec::SetLowerBoundPartitionKey(const Slice& partitionKey) { - if (partitionKey.compare(lower_bound_partition_key_) > 0) { - lower_bound_partition_key_ = partitionKey.ToString(); +void ScanSpec::SetLowerBoundPartitionKey(const Slice& partition_key) { + if (partition_key.compare(lower_bound_partition_key_) > 0) { + lower_bound_partition_key_ = partition_key.ToString(); } } -void ScanSpec::SetExclusiveUpperBoundPartitionKey(const Slice& partitionKey) { +void ScanSpec::SetExclusiveUpperBoundPartitionKey(const Slice& partition_key) { if (exclusive_upper_bound_partition_key_.empty() || - (!partitionKey.empty() && partitionKey.compare(exclusive_upper_bound_partition_key_) < 0)) { - exclusive_upper_bound_partition_key_ = partitionKey.ToString(); + (!partition_key.empty() && partition_key.compare(exclusive_upper_bound_partition_key_) < 0)) { + exclusive_upper_bound_partition_key_ = partition_key.ToString(); } } http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/experiments/rwlock-perf.cc ---------------------------------------------------------------------- diff --git a/src/kudu/experiments/rwlock-perf.cc b/src/kudu/experiments/rwlock-perf.cc index 55a2d77..a419c32 100644 --- a/src/kudu/experiments/rwlock-perf.cc +++ b/src/kudu/experiments/rwlock-perf.cc @@ -46,19 +46,19 @@ class my_spinlock : public boost::detail::spinlock { DISALLOW_COPY_AND_ASSIGN(my_spinlock); }; -struct per_cpu_lock { - struct padded_lock { +struct PerCpuLock { + struct PaddedLock { my_spinlock lock; char padding[CACHELINE_SIZE - sizeof(my_spinlock)]; }; - per_cpu_lock() { + PerCpuLock() { n_cpus_ = base::NumCPUs(); CHECK_GT(n_cpus_, 0); - locks_ = new padded_lock[n_cpus_]; + locks_ = new PaddedLock[n_cpus_]; } - ~per_cpu_lock() { + ~PerCpuLock() { delete [] locks_; } @@ -69,11 +69,11 @@ struct per_cpu_lock { } int n_cpus_; - padded_lock *locks_; + PaddedLock *locks_; }; -struct shared_data { +struct SharedData { kudu::rw_spinlock rw_spinlock; kudu::RWMutex rwlock; std::mutex lock; @@ -81,7 +81,7 @@ struct shared_data { }; -class noop_lock { +class NoopLock { public: void lock() {} void unlock() {} @@ -106,7 +106,7 @@ static void depend_on(float val) { } } -void shared_rwlock_entry(shared_data *shared) { +void shared_rwlock_entry(SharedData *shared) { float result = 1; for (int i = 0; i < 1000000; i++) { shared->rwlock.lock_shared(); @@ -116,7 +116,7 @@ void shared_rwlock_entry(shared_data *shared) { depend_on(result); } -void shared_rw_spinlock_entry(shared_data *shared) { +void shared_rw_spinlock_entry(SharedData *shared) { float result = 1; for (int i = 0; i < 1000000; i++) { shared->rw_spinlock.lock_shared(); @@ -126,7 +126,7 @@ void shared_rw_spinlock_entry(shared_data *shared) { depend_on(result); } -void shared_mutex_entry(shared_data *shared) { +void shared_mutex_entry(SharedData *shared) { float result = 1; for (int i = 0; i < 1000000; i++) { shared->lock.lock(); @@ -149,7 +149,7 @@ void own_mutex_entry() { depend_on(result); } -void percpu_rwlock_entry(shared_data *shared) { +void percpu_rwlock_entry(SharedData *shared) { float result = 1; for (int i = 0; i < 1000000; i++) { kudu::rw_spinlock &l = shared->per_cpu.get_lock(); @@ -174,7 +174,7 @@ enum TestMethod { void test_shared_lock(int num_threads, TestMethod method, const char *name) { vector<thread> threads; - shared_data shared; + SharedData shared; for (int i = 0; i < num_threads; i++) { switch (method) { @@ -191,7 +191,7 @@ void test_shared_lock(int num_threads, TestMethod method, const char *name) { threads.emplace_back(own_mutex_entry<my_spinlock>); break; case NO_LOCK: - threads.emplace_back(own_mutex_entry<noop_lock>); + threads.emplace_back(own_mutex_entry<NoopLock>); break; case PERCPU_RWLOCK: threads.emplace_back(percpu_rwlock_entry, &shared); http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/gutil/hash/hash.cc ---------------------------------------------------------------------- diff --git a/src/kudu/gutil/hash/hash.cc b/src/kudu/gutil/hash/hash.cc index 92a8ca2..71141c0 100644 --- a/src/kudu/gutil/hash/hash.cc +++ b/src/kudu/gutil/hash/hash.cc @@ -187,7 +187,7 @@ uint64 FingerprintInterleavedImplementation(const char *s, uint32 len) { #if defined(__GNUC__) #include <ext/hash_set> -namespace __gnu_cxx { +namespace __gnu_cxx { // NOLINT(*) template class hash_set<std::string>; template class hash_map<std::string, std::string>; http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/gutil/spinlock.cc ---------------------------------------------------------------------- diff --git a/src/kudu/gutil/spinlock.cc b/src/kudu/gutil/spinlock.cc index 8a02c95..63b8513 100644 --- a/src/kudu/gutil/spinlock.cc +++ b/src/kudu/gutil/spinlock.cc @@ -57,8 +57,8 @@ const base::LinkerInitialized SpinLock::LINKER_INITIALIZED = base::LINKER_INITIALIZED; namespace { -struct SpinLock_InitHelper { - SpinLock_InitHelper() { +struct SpinLockInitHelper { + SpinLockInitHelper() { // On multi-cpu machines, spin for longer before yielding // the processor or sleeping. Reduces idle time significantly. if (base::NumCPUs() > 1) { @@ -70,7 +70,7 @@ struct SpinLock_InitHelper { // Hook into global constructor execution: // We do not do adaptive spinning before that, // but nothing lock-intensive should be going on at that time. -static SpinLock_InitHelper init_helper; +static SpinLockInitHelper init_helper; } // unnamed namespace http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/integration-tests/full_stack-insert-scan-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/integration-tests/full_stack-insert-scan-test.cc b/src/kudu/integration-tests/full_stack-insert-scan-test.cc index c2160af..abd3ea2 100644 --- a/src/kudu/integration-tests/full_stack-insert-scan-test.cc +++ b/src/kudu/integration-tests/full_stack-insert-scan-test.cc @@ -105,7 +105,7 @@ class FullStackInsertScanTest : public KuduTest { kNumInsertClients(DefaultFlag(FLAGS_concurrent_inserts, 3, 10)), kNumInsertsPerClient(DefaultFlag(FLAGS_inserts_per_client, 500, 50000)), kNumRows(kNumInsertClients * kNumInsertsPerClient), - kFlushEveryN(DefaultFlag(FLAGS_rows_per_batch, 125, 5000)), + flush_every_n_(DefaultFlag(FLAGS_rows_per_batch, 125, 5000)), random_(SeedRandom()), sessions_(kNumInsertClients), tables_(kNumInsertClients) { @@ -204,7 +204,7 @@ class FullStackInsertScanTest : public KuduTest { kInt32ColBase, kInt64ColBase = kInt32ColBase + kNumIntCols }; - const int kFlushEveryN; + const int flush_every_n_; Random random_; @@ -365,7 +365,7 @@ void FullStackInsertScanTest::InsertRows(CountDownLatch* start_latch, int id, // Report updates or flush every so often, using the synchronizer to always // start filling up the next batch while previous one is sent out. - if (key % kFlushEveryN == 0) { + if (key % flush_every_n_ == 0) { Status s = sync.Wait(); if (!s.ok()) { LogSessionErrorsAndDie(session, s); http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/tools/ksck_remote.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tools/ksck_remote.cc b/src/kudu/tools/ksck_remote.cc index b9f8eae..0a3d8e4 100644 --- a/src/kudu/tools/ksck_remote.cc +++ b/src/kudu/tools/ksck_remote.cc @@ -135,7 +135,7 @@ class ChecksumStepper; class ChecksumCallbackHandler { public: explicit ChecksumCallbackHandler(ChecksumStepper* const stepper) - : stepper(DCHECK_NOTNULL(stepper)) { + : stepper_(DCHECK_NOTNULL(stepper)) { } // Invoked by an RPC completion callback. Simply calls back into the stepper. @@ -143,7 +143,7 @@ class ChecksumCallbackHandler { void Run(); private: - ChecksumStepper* const stepper; + ChecksumStepper* const stepper_; }; // Simple class to have a "conversation" over multiple requests to a server @@ -267,7 +267,7 @@ class ChecksumStepper { }; void ChecksumCallbackHandler::Run() { - stepper->HandleResponse(); + stepper_->HandleResponse(); delete this; } http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/util/memcmpable_varint.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/memcmpable_varint.cc b/src/kudu/util/memcmpable_varint.cc index e55addf..35314f8 100644 --- a/src/kudu/util/memcmpable_varint.cc +++ b/src/kudu/util/memcmpable_varint.cc @@ -193,45 +193,45 @@ static size_t sqlite4PutVarint64(uint8_t *z, uint64_t x) { static int sqlite4GetVarint64( const uint8_t *z, int n, - uint64_t *pResult) { + uint64_t *p_result) { unsigned int x; if ( n < 1) return 0; if (z[0] <= 240) { - *pResult = z[0]; + *p_result = z[0]; return 1; } if (z[0] <= 248) { if ( n < 2) return 0; - *pResult = (z[0]-241)*256 + z[1] + 240; + *p_result = (z[0]-241)*256 + z[1] + 240; return 2; } if (n < z[0]-246 ) return 0; if (z[0] == 249) { - *pResult = 2288 + 256*z[1] + z[2]; + *p_result = 2288 + 256*z[1] + z[2]; return 3; } if (z[0] == 250) { - *pResult = (z[1]<<16) + (z[2]<<8) + z[3]; + *p_result = (z[1]<<16) + (z[2]<<8) + z[3]; return 4; } x = (z[1]<<24) + (z[2]<<16) + (z[3]<<8) + z[4]; if (z[0] == 251) { - *pResult = x; + *p_result = x; return 5; } if (z[0] == 252) { - *pResult = (((uint64_t)x)<<8) + z[5]; + *p_result = (((uint64_t)x)<<8) + z[5]; return 6; } if (z[0] == 253) { - *pResult = (((uint64_t)x)<<16) + (z[5]<<8) + z[6]; + *p_result = (((uint64_t)x)<<16) + (z[5]<<8) + z[6]; return 7; } if (z[0] == 254) { - *pResult = (((uint64_t)x)<<24) + (z[5]<<16) + (z[6]<<8) + z[7]; + *p_result = (((uint64_t)x)<<24) + (z[5]<<16) + (z[6]<<8) + z[7]; return 8; } - *pResult = (((uint64_t)x)<<32) + + *p_result = (((uint64_t)x)<<32) + (0xffffffff & ((z[5]<<24) + (z[6]<<16) + (z[7]<<8) + z[8])); return 9; } http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/util/memory/memory.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/memory/memory.cc b/src/kudu/util/memory/memory.cc index 9db0464..e393540 100644 --- a/src/kudu/util/memory/memory.cc +++ b/src/kudu/util/memory/memory.cc @@ -147,23 +147,23 @@ void* HeapBufferAllocator::Malloc(size_t size) { } } -void* HeapBufferAllocator::Realloc(void* previousData, size_t previousSize, - size_t newSize) { +void* HeapBufferAllocator::Realloc(void* previous_data, size_t previous_size, + size_t new_size) { if (aligned_mode_) { - void* data = Malloc(newSize); + void* data = Malloc(new_size); if (data) { // NOTE(ptab): We should use realloc here to avoid memmory coping, // but it doesn't work on memory allocated by posix_memalign(...). // realloc reallocates the memory but doesn't preserve the content. // TODO(ptab): reiterate after some time to check if it is fixed (tcmalloc ?) - memcpy(data, previousData, min(previousSize, newSize)); - free(previousData); + memcpy(data, previous_data, min(previous_size, new_size)); + free(previous_data); return data; } else { return nullptr; } } else { - return realloc(previousData, newSize); + return realloc(previous_data, new_size); } } http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/util/net/socket.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/net/socket.cc b/src/kudu/util/net/socket.cc index e0bea14..ac28b64 100644 --- a/src/kudu/util/net/socket.cc +++ b/src/kudu/util/net/socket.cc @@ -255,10 +255,10 @@ Status Socket::SetReuseAddr(bool flag) { } Status Socket::BindAndListen(const Sockaddr &sockaddr, - int listenQueueSize) { + int listen_queue_size) { RETURN_NOT_OK(SetReuseAddr(true)); RETURN_NOT_OK(Bind(sockaddr)); - RETURN_NOT_OK(Listen(listenQueueSize)); + RETURN_NOT_OK(Listen(listen_queue_size)); return Status::OK(); } http://git-wip-us.apache.org/repos/asf/kudu/blob/850ebef6/src/kudu/util/striped64.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/striped64.cc b/src/kudu/util/striped64.cc index 8343177..cf6c780 100644 --- a/src/kudu/util/striped64.cc +++ b/src/kudu/util/striped64.cc @@ -130,11 +130,11 @@ void Striped64::RetryUpdate(int64_t x, Rehash to_rehash) { tls_hashcode_ = h; } -void Striped64::InternalReset(int64_t initialValue) { +void Striped64::InternalReset(int64_t initial_value) { const int32_t n = base::subtle::Acquire_Load(&num_cells_); - base_.value_.Store(initialValue); + base_.value_.Store(initial_value); for (int i = 0; i < n; i++) { - cells_[i].value_.Store(initialValue); + cells_[i].value_.Store(initial_value); } }
