You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2021/03/08 06:44:09 UTC

[kudu] branch master updated: [gutil] use C++17's [[fallthrough]] attribute

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new c89ec08  [gutil] use C++17's [[fallthrough]] attribute
c89ec08 is described below

commit c89ec08d6a9e92f15d7c6846fae57f82d0616eb8
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Sun Mar 7 10:52:53 2021 -0800

    [gutil] use C++17's [[fallthrough]] attribute
    
    Since switching to C++17-compatible compiler, it's possible to use
    the [[fallthrough]] attribute.  This patch does exactly so, in addition
    removing now obsolete FALLTHROUGH_INTENDED macro.
    
    Change-Id: Icd0532c0c6403f9bccfa59413a103d812633f4f6
    Reviewed-on: http://gerrit.cloudera.org:8080/17156
    Tested-by: Kudu Jenkins
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
 src/kudu/common/column_predicate.cc                | 67 ++++++++++------------
 src/kudu/consensus/consensus_peers.cc              |  2 +-
 src/kudu/consensus/quorum_util.cc                  |  3 +-
 src/kudu/fs/dir_manager.cc                         |  2 +-
 src/kudu/gutil/macros.h                            | 42 --------------
 src/kudu/integration-tests/fuzz-itest.cc           |  4 +-
 .../integration-tests/raft_consensus-itest-base.cc |  3 +-
 .../tombstoned_voting-stress-test.cc               |  5 +-
 src/kudu/master/master_service.cc                  |  4 +-
 src/kudu/tablet/delta_stats.cc                     | 33 ++++++-----
 src/kudu/tablet/ops/op_driver.cc                   |  4 +-
 src/kudu/tablet/tablet_replica.cc                  |  2 +-
 src/kudu/tserver/tablet_service.cc                 |  2 +-
 src/kudu/util/threadpool.cc                        |  3 +-
 14 files changed, 60 insertions(+), 116 deletions(-)

diff --git a/src/kudu/common/column_predicate.cc b/src/kudu/common/column_predicate.cc
index 99c50e8..aabd37a 100644
--- a/src/kudu/common/column_predicate.cc
+++ b/src/kudu/common/column_predicate.cc
@@ -29,7 +29,6 @@
 #include "kudu/common/rowblock.h"
 #include "kudu/common/schema.h"
 #include "kudu/common/types.h"
-#include "kudu/gutil/macros.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/join.h"
 #include "kudu/gutil/strings/substitute.h"
@@ -328,16 +327,14 @@ void ColumnPredicate::MergeIntoRange(const ColumnPredicate& other) {
   CHECK(predicate_type_ == PredicateType::Range);
 
   switch (other.predicate_type()) {
-    case PredicateType::None: {
+    case PredicateType::None:
       SetToNone();
       return;
-    };
-    case PredicateType::InBloomFilter: {
+    case PredicateType::InBloomFilter:
       bloom_filters_ = other.bloom_filters_;
       predicate_type_ = PredicateType::InBloomFilter;
-      FALLTHROUGH_INTENDED;
-    }
-    case PredicateType::Range: {
+      [[fallthrough]];
+    case PredicateType::Range:
       // Set the lower bound to the larger of the two.
       if (other.lower_ != nullptr &&
           (lower_ == nullptr || column_.type_info()->Compare(lower_, other.lower_) < 0)) {
@@ -352,8 +349,7 @@ void ColumnPredicate::MergeIntoRange(const ColumnPredicate& other) {
 
       Simplify();
       return;
-    };
-    case PredicateType::Equality: {
+    case PredicateType::Equality:
       if ((lower_ != nullptr && column_.type_info()->Compare(lower_, other.lower_) > 0) ||
           (upper_ != nullptr && column_.type_info()->Compare(upper_, other.lower_) <= 0)) {
         // The equality value does not fall in this range.
@@ -364,13 +360,12 @@ void ColumnPredicate::MergeIntoRange(const ColumnPredicate& other) {
         upper_ = nullptr;
       }
       return;
-    };
-    case PredicateType::IsNotNull: return;
-    case PredicateType::IsNull: {
+    case PredicateType::IsNotNull:
+      return;
+    case PredicateType::IsNull:
       SetToNone();
       return;
-    };
-    case PredicateType::InList : {
+    case PredicateType::InList:
       // The InList predicate values are examined to check whether
       // they lie in the range.
       // The current predicate is then converted from a range predicate to
@@ -388,7 +383,6 @@ void ColumnPredicate::MergeIntoRange(const ColumnPredicate& other) {
       predicate_type_ = PredicateType::InList;
       Simplify();
       return;
-    };
   }
   LOG(FATAL) << "unknown predicate type";
 }
@@ -591,16 +585,14 @@ void ColumnPredicate::MergeIntoBloomFilter(const ColumnPredicate &other) {
   DCHECK(!bloom_filters_.empty());
 
   switch (other.predicate_type()) {
-    case PredicateType::None: {
+    case PredicateType::None:
       SetToNone();
       return;
-    };
-    case PredicateType::InBloomFilter: {
+    case PredicateType::InBloomFilter:
       bloom_filters_.insert(bloom_filters_.end(), other.bloom_filters().begin(),
                             other.bloom_filters().end());
-      FALLTHROUGH_INTENDED;
-    }
-    case PredicateType::Range: {
+      [[fallthrough]];
+    case PredicateType::Range:
       // Merge the optional lower and upper bound.
       if (other.lower_ != nullptr &&
           (lower_ == nullptr || column_.type_info()->Compare(lower_, other.lower_) < 0)) {
@@ -612,8 +604,7 @@ void ColumnPredicate::MergeIntoBloomFilter(const ColumnPredicate &other) {
       }
       Simplify();
       return;
-    }
-    case PredicateType::Equality: {
+    case PredicateType::Equality:
       if (CheckValueInBloomFilter(other.lower_)) {
         // Value falls in bloom filters so change to Equality predicate.
         predicate_type_ = PredicateType::Equality;
@@ -624,13 +615,12 @@ void ColumnPredicate::MergeIntoBloomFilter(const ColumnPredicate &other) {
         SetToNone(); // Value does not fall in bloom filters.
       }
       return;
-    }
-    case PredicateType::IsNotNull: return;
-    case PredicateType::IsNull: {
+    case PredicateType::IsNotNull:
+      return;
+    case PredicateType::IsNull:
       SetToNone();
       return;
-    }
-    case PredicateType::InList: {
+    case PredicateType::InList:
       DCHECK(other.values_.size() > 1);
       std::vector<const void*> new_values;
       std::copy_if(other.values_.begin(), other.values_.end(), std::back_inserter(new_values),
@@ -642,7 +632,6 @@ void ColumnPredicate::MergeIntoBloomFilter(const ColumnPredicate &other) {
       bloom_filters_.clear();
       Simplify();
       return;
-    }
   }
   LOG(FATAL) << "unknown predicate type";
 }
@@ -884,8 +873,9 @@ bool ColumnPredicate::operator==(const ColumnPredicate& other) const {
     return false;
   }
   switch (predicate_type_) {
-    case PredicateType::Equality: return column_.type_info()->Compare(lower_, other.lower_) == 0;
-    case PredicateType::InBloomFilter: {
+    case PredicateType::Equality:
+      return column_.type_info()->Compare(lower_, other.lower_) == 0;
+    case PredicateType::InBloomFilter:
       if (bloom_filters_.size() != other.bloom_filters().size()) {
         return false;
       }
@@ -897,25 +887,26 @@ bool ColumnPredicate::operator==(const ColumnPredicate& other) const {
                       })) {
         return false;
       }
-      FALLTHROUGH_INTENDED;
-    };
+      [[fallthrough]];
     case PredicateType::Range: {
       auto bound_equal = [&] (const void* eleml, const void* elemr) {
           return (eleml == elemr || (eleml != nullptr && elemr != nullptr &&
                                      column_.type_info()->Compare(eleml, elemr) == 0));
       };
       return bound_equal(lower_, other.lower_) && bound_equal(upper_, other.upper_);
-    };
-    case PredicateType::InList: {
-      if (values_.size() != other.values_.size()) return false;
+    }
+    case PredicateType::InList:
+      if (values_.size() != other.values_.size()) {
+        return false;
+      }
       for (int i = 0; i < values_.size(); i++) {
         if (column_.type_info()->Compare(values_[i], other.values_[i]) != 0) return false;
       }
       return true;
-    };
     case PredicateType::None:
     case PredicateType::IsNotNull:
-    case PredicateType::IsNull: return true;
+    case PredicateType::IsNull:
+      return true;
   }
   LOG(FATAL) << "unknown predicate type";
 }
diff --git a/src/kudu/consensus/consensus_peers.cc b/src/kudu/consensus/consensus_peers.cc
index fd75076..758a95d 100644
--- a/src/kudu/consensus/consensus_peers.cc
+++ b/src/kudu/consensus/consensus_peers.cc
@@ -360,7 +360,7 @@ void Peer::ProcessResponse() {
     TabletServerErrorPB resp_error = response_.error();
     switch (response_.error().code()) {
       // We treat WRONG_SERVER_UUID as failed.
-      case TabletServerErrorPB::WRONG_SERVER_UUID: FALLTHROUGH_INTENDED;
+      case TabletServerErrorPB::WRONG_SERVER_UUID: [[fallthrough]];
       case TabletServerErrorPB::TABLET_FAILED:
         ps = PeerStatus::TABLET_FAILED;
         break;
diff --git a/src/kudu/consensus/quorum_util.cc b/src/kudu/consensus/quorum_util.cc
index 07e3fd9..7b7524b 100644
--- a/src/kudu/consensus/quorum_util.cc
+++ b/src/kudu/consensus/quorum_util.cc
@@ -28,7 +28,6 @@
 #include <glog/logging.h>
 
 #include "kudu/common/common.pb.h"
-#include "kudu/gutil/macros.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/join.h"
@@ -563,7 +562,7 @@ bool ShouldEvictReplica(const RaftConfigPB& config,
       case HealthReportPB::HEALTHY:
         priority = 0;
         break;
-      case HealthReportPB::UNKNOWN:   FALLTHROUGH_INTENDED;
+      case HealthReportPB::UNKNOWN:   [[fallthrough]];
       default:
         priority = 1;
         break;
diff --git a/src/kudu/fs/dir_manager.cc b/src/kudu/fs/dir_manager.cc
index fd8a81f..6a47ed9 100644
--- a/src/kudu/fs/dir_manager.cc
+++ b/src/kudu/fs/dir_manager.cc
@@ -125,7 +125,7 @@ Status Dir::RefreshAvailableSpace(RefreshMode mode) {
       if (MonoTime::Now() < expiry) {
         break;
       }
-      FALLTHROUGH_INTENDED; // Root was previously full, check again.
+      [[fallthrough]]; // Root was previously full, check again.
     }
     case RefreshMode::ALWAYS: {
       int64_t available_bytes_new;
diff --git a/src/kudu/gutil/macros.h b/src/kudu/gutil/macros.h
index 7d0f53a..f84d1e6 100644
--- a/src/kudu/gutil/macros.h
+++ b/src/kudu/gutil/macros.h
@@ -217,48 +217,6 @@ namespace base {
 enum LinkerInitialized { LINKER_INITIALIZED };
 }
 
-// The FALLTHROUGH_INTENDED macro can be used to annotate implicit fall-through
-// between switch labels:
-//  switch (x) {
-//    case 40:
-//    case 41:
-//      if (truth_is_out_there) {
-//        ++x;
-//        FALLTHROUGH_INTENDED;  // Use instead of/along with annotations in
-//                               // comments.
-//      } else {
-//        return x;
-//      }
-//    case 42:
-//      ...
-//
-//  As shown in the example above, the FALLTHROUGH_INTENDED macro should be
-//  followed by a semicolon. It is designed to mimic control-flow statements
-//  like 'break;', so it can be placed in most places where 'break;' can, but
-//  only if there are no statements on the execution path between it and the
-//  next switch label.
-//
-//  When compiled with clang in C++11 mode, the FALLTHROUGH_INTENDED macro is
-//  expanded to [[clang::fallthrough]] attribute, which is analysed when
-//  performing switch labels fall-through diagnostic ('-Wimplicit-fallthrough').
-//  See clang documentation on language extensions for details:
-//  http://clang.llvm.org/docs/LanguageExtensions.html#clang__fallthrough
-//
-//  When used with unsupported compilers, the FALLTHROUGH_INTENDED macro has no
-//  effect on diagnostics.
-//
-//  In either case this macro has no effect on runtime behavior and performance
-//  of code.
-#if defined(__clang__) && defined(LANG_CXX11) && defined(__has_warning)
-#if __has_feature(cxx_attributes) && __has_warning("-Wimplicit-fallthrough")
-#define FALLTHROUGH_INTENDED [[clang::fallthrough]]  // NOLINT
-#endif
-#endif
-
-#ifndef FALLTHROUGH_INTENDED
-#define FALLTHROUGH_INTENDED do { } while (0)
-#endif
-
 // Retry on EINTR for functions like read() that return -1 on error.
 #define RETRY_ON_EINTR(err, expr) do { \
   static_assert(std::is_signed<decltype(err)>::value, \
diff --git a/src/kudu/integration-tests/fuzz-itest.cc b/src/kudu/integration-tests/fuzz-itest.cc
index 86646a5..9a5b0c1 100644
--- a/src/kudu/integration-tests/fuzz-itest.cc
+++ b/src/kudu/integration-tests/fuzz-itest.cc
@@ -1206,7 +1206,7 @@ void FuzzTest::ValidateFuzzCase(const vector<TestOp>& test_ops) {
       case TEST_INSERT:
       case TEST_INSERT_PK_ONLY:
         CHECK(!exists[test_op.val]) << "invalid case: inserting already-existing row";
-        FALLTHROUGH_INTENDED;
+        [[fallthrough]];
       case TEST_INSERT_IGNORE:
       case TEST_INSERT_IGNORE_PK_ONLY: {
         const auto& txn_id = test_op.val2;
@@ -1249,7 +1249,7 @@ void FuzzTest::ValidateFuzzCase(const vector<TestOp>& test_ops) {
             case TEST_INSERT:
             case TEST_INSERT_PK_ONLY:
               CHECK(!exists[row]);
-              FALLTHROUGH_INTENDED;
+              [[fallthrough]];
             case TEST_INSERT_IGNORE:
             case TEST_INSERT_IGNORE_PK_ONLY:
               exists[row] = true;
diff --git a/src/kudu/integration-tests/raft_consensus-itest-base.cc b/src/kudu/integration-tests/raft_consensus-itest-base.cc
index c8e420b..f84a71c 100644
--- a/src/kudu/integration-tests/raft_consensus-itest-base.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest-base.cc
@@ -38,7 +38,6 @@
 #include "kudu/common/wire_protocol.h"
 #include "kudu/consensus/consensus.pb.h"
 #include "kudu/consensus/opid.pb.h"
-#include "kudu/gutil/macros.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/stringprintf.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
@@ -227,7 +226,7 @@ void RaftConsensusITestBase::CauseSpecificFollowerToFallBehindLogGC(
     case BehindWalGcBehavior::STOP_CONTINUE:
       ASSERT_OK(replica_ets->Pause());
       break;
-    case BehindWalGcBehavior::SHUTDOWN_RESTART: FALLTHROUGH_INTENDED;
+    case BehindWalGcBehavior::SHUTDOWN_RESTART: [[fallthrough]];
     case BehindWalGcBehavior::SHUTDOWN:
       replica_ets->Shutdown();
       break;
diff --git a/src/kudu/integration-tests/tombstoned_voting-stress-test.cc b/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
index 41496f0..a092a71 100644
--- a/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
+++ b/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
@@ -34,7 +34,6 @@
 #include "kudu/common/wire_protocol.pb.h"
 #include "kudu/consensus/consensus.pb.h"
 #include "kudu/consensus/opid.pb.h"
-#include "kudu/gutil/macros.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
 #include "kudu/integration-tests/mini_cluster_fs_inspector.h"
@@ -181,7 +180,7 @@ void TombstonedVotingStressTest::RunVoteRequestLoop() {
                                   /*ignore_live_leader=*/ true, /*is_pre_election=*/ false,
                                   kTimeout);
     switch (state) {
-      case kRunning: FALLTHROUGH_INTENDED;
+      case kRunning: [[fallthrough]];
       case kTombstoned:
         // We should always be able to vote in this case.
         if (s.ok()) {
@@ -195,7 +194,7 @@ void TombstonedVotingStressTest::RunVoteRequestLoop() {
       // because there is a small window of time where we have stopped
       // RaftConsensus but we haven't yet recorded the last-logged opid in the
       // tablet metadata.
-      case kTombstoning: FALLTHROUGH_INTENDED;
+      case kTombstoning: [[fallthrough]];
       case kCopying:
         if (s.ok()) {
           LOG(INFO) << "Vote OK: state = " << state;
diff --git a/src/kudu/master/master_service.cc b/src/kudu/master/master_service.cc
index 697cdf8..a596b0e 100644
--- a/src/kudu/master/master_service.cc
+++ b/src/kudu/master/master_service.cc
@@ -842,8 +842,8 @@ void MasterServiceImpl::RefreshAuthzCache(
 
 bool MasterServiceImpl::SupportsFeature(uint32_t feature) const {
   switch (feature) {
-    case MasterFeatures::RANGE_PARTITION_BOUNDS:    FALLTHROUGH_INTENDED;
-    case MasterFeatures::ADD_DROP_RANGE_PARTITIONS: FALLTHROUGH_INTENDED;
+    case MasterFeatures::RANGE_PARTITION_BOUNDS:    [[fallthrough]];
+    case MasterFeatures::ADD_DROP_RANGE_PARTITIONS: [[fallthrough]];
     case MasterFeatures::REPLICA_MANAGEMENT:
       return true;
     case MasterFeatures::GENERATE_AUTHZ_TOKEN:
diff --git a/src/kudu/tablet/delta_stats.cc b/src/kudu/tablet/delta_stats.cc
index 3a21dc9..bab7a1e 100644
--- a/src/kudu/tablet/delta_stats.cc
+++ b/src/kudu/tablet/delta_stats.cc
@@ -29,13 +29,11 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/tablet/tablet.pb.h"
 
-using strings::Substitute;
-
-namespace kudu {
-
 using std::string;
 using std::vector;
+using strings::Substitute;
 
+namespace kudu {
 namespace tablet {
 
 DeltaStats::DeltaStats()
@@ -65,23 +63,25 @@ Status DeltaStats::UpdateStats(const Timestamp& timestamp,
   RowChangeListDecoder decoder(update);
   RETURN_NOT_OK(decoder.Init());
   switch (decoder.get_type()) {
-    case RowChangeList::kReinsert: {
+    case RowChangeList::kReinsert:
       IncrReinsertCount(1);
-    } // FALLTHROUGH INTENDED. REINSERTs contain column updates so we need to account
-      // for those in the updated column stats.
-    case RowChangeList::kUpdate: {
-      vector<ColumnId> col_ids;
-      RETURN_NOT_OK(decoder.GetIncludedColumnIds(&col_ids));
-      for (const ColumnId& col_id : col_ids) {
-        IncrUpdateCount(col_id, 1);
+      [[fallthrough]];
+      // FALLTHROUGH INTENDED: REINSERTs contain column updates so we need
+      // to account for those in the updated column stats.
+    case RowChangeList::kUpdate:
+      {
+        vector<ColumnId> col_ids;
+        RETURN_NOT_OK(decoder.GetIncludedColumnIds(&col_ids));
+        for (const ColumnId& col_id : col_ids) {
+          IncrUpdateCount(col_id, 1);
+        }
       }
       break;
-    }
-    case RowChangeList::kDelete: {
+    case RowChangeList::kDelete:
       IncrDeleteCount(1);
       break;
-    }
-    default: LOG(FATAL) << "Invalid mutation type: " << decoder.get_type();
+    default:
+      LOG(FATAL) << "Invalid mutation type: " << decoder.get_type();
   }
 
   if (min_timestamp_ > timestamp) {
@@ -148,6 +148,5 @@ void DeltaStats::AddColumnIdsWithUpdates(std::set<ColumnId>* col_ids) const {
   }
 }
 
-
 } // namespace tablet
 } // namespace kudu
diff --git a/src/kudu/tablet/ops/op_driver.cc b/src/kudu/tablet/ops/op_driver.cc
index 554e013..d717c00 100644
--- a/src/kudu/tablet/ops/op_driver.cc
+++ b/src/kudu/tablet/ops/op_driver.cc
@@ -339,7 +339,7 @@ Status OpDriver::Prepare() {
     }
     case REPLICATION_FAILED:
       DCHECK(!op_status_.ok());
-      FALLTHROUGH_INTENDED;
+      [[fallthrough]];
     case REPLICATED:
     {
       // We can move on to apply.
@@ -376,7 +376,7 @@ void OpDriver::HandleFailure(const Status& s) {
             << ": " << op_status_.ToString()
             << " op:" << ToString();
       }
-      FALLTHROUGH_INTENDED;
+      [[fallthrough]];
     }
     case NOT_REPLICATING:
     case REPLICATION_FAILED:
diff --git a/src/kudu/tablet/tablet_replica.cc b/src/kudu/tablet/tablet_replica.cc
index cd61d16..f4995c5 100644
--- a/src/kudu/tablet/tablet_replica.cc
+++ b/src/kudu/tablet/tablet_replica.cc
@@ -472,7 +472,7 @@ void TabletReplica::set_state(TabletStatePB new_state) {
     case STOPPED:
       CHECK_EQ(STOPPING, state_);
       break;
-    case SHUTDOWN: FALLTHROUGH_INTENDED;
+    case SHUTDOWN: [[fallthrough]];
     case FAILED:
       CHECK_EQ(STOPPED, state_) << TabletStatePB_Name(state_);
       break;
diff --git a/src/kudu/tserver/tablet_service.cc b/src/kudu/tserver/tablet_service.cc
index 9385608..c2d8c5e 100644
--- a/src/kudu/tserver/tablet_service.cc
+++ b/src/kudu/tserver/tablet_service.cc
@@ -1203,7 +1203,7 @@ Status ValidateCoordinatorOpFields(const CoordinatorOpPB& op) {
         return Status::InvalidArgument(Substitute("Missing participant id: $0",
                                                   SecureShortDebugString(op)));
       }
-      FALLTHROUGH_INTENDED;
+      [[fallthrough]];
     case CoordinatorOpPB::BEGIN_TXN:
     case CoordinatorOpPB::BEGIN_COMMIT_TXN:
     case CoordinatorOpPB::ABORT_TXN:
diff --git a/src/kudu/util/threadpool.cc b/src/kudu/util/threadpool.cc
index 569c968..9f413ad 100644
--- a/src/kudu/util/threadpool.cc
+++ b/src/kudu/util/threadpool.cc
@@ -28,7 +28,6 @@
 
 #include <glog/logging.h>
 
-#include "kudu/gutil/macros.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/sysinfo.h"
@@ -164,7 +163,7 @@ void ThreadPoolToken::Shutdown() {
         break;
       }
       Transition(State::QUIESCING);
-      FALLTHROUGH_INTENDED;
+      [[fallthrough]];
     case State::QUIESCING:
       // The token is already quiescing. Just wait for a worker thread to
       // switch it to QUIESCED.