You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2024/01/04 05:24:50 UTC
[Impala-ASF-CR] IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20847
Change subject: IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
......................................................................
IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
In switch/case statements, one case can fallthrough to the
next case. Sometimes this is intentional, but it is also a
common source of bugs (i.e. a missing break/return statement).
Clang-Tidy's clang-diagnostic-implicit-fallthrough flags
locations where a case falls through to the next case without
an explicit fallthrough declaration.
This change enables clang-diagnostic-implicit-fallthrough and
fixes failing locations. Since Impala uses C++17, this uses
C++17's [[fallthrough]] to indicate an explicit fallthrough.
This also adjusts clang-tidy's output to suggest [[fallthrough]]
as the preferred way to indicate fallthrough.
Testing:
- Ran core job
- Ran clang tidy
Change-Id: I6d65c92b442fa0317c3af228997571e124a54092
---
M .clang-tidy
M be/src/benchmarks/atoi-benchmark.cc
M be/src/codegen/codegen-anyval-read-write-info.cc
M be/src/codegen/codegen-anyval.cc
M be/src/exec/orc/hdfs-orc-scanner.cc
M be/src/exec/parquet/parquet-column-stats.cc
M be/src/exprs/agg-fn-evaluator.cc
M be/src/exprs/anyval-util.h
M be/src/runtime/datetime-parser-common.cc
M be/src/runtime/descriptors.cc
M be/src/runtime/io/disk-file.cc
M be/src/runtime/raw-value.cc
M be/src/runtime/raw-value.inline.h
M be/src/runtime/runtime-filter-ir.cc
M be/src/runtime/types.cc
M be/src/service/query-result-set.cc
M be/src/util/bit-packing.inline.h
M be/src/util/hash-util.h
M be/src/util/jwt-util.cc
M be/src/util/redactor.cc
M be/src/util/string-parser.h
M bin/run_clang_tidy.sh
22 files changed, 105 insertions(+), 32 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/20847/1
--
To view, visit http://gerrit.cloudera.org:8080/20847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6d65c92b442fa0317c3af228997571e124a54092
Gerrit-Change-Number: 20847
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20847 )
Change subject: IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d65c92b442fa0317c3af228997571e124a54092
Gerrit-Change-Number: 20847
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 04 Jan 2024 19:29:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20847 )
Change subject: IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/20847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d65c92b442fa0317c3af228997571e124a54092
Gerrit-Change-Number: 20847
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 04 Jan 2024 09:58:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20847 )
Change subject: IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14868/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d65c92b442fa0317c3af228997571e124a54092
Gerrit-Change-Number: 20847
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Jan 2024 05:52:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20847 )
Change subject: IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
......................................................................
IMPALA-12668: Enable clang-tidy checks for implicit fallthrough
In switch/case statements, one case can fallthrough to the
next case. Sometimes this is intentional, but it is also a
common source of bugs (i.e. a missing break/return statement).
Clang-Tidy's clang-diagnostic-implicit-fallthrough flags
locations where a case falls through to the next case without
an explicit fallthrough declaration.
This change enables clang-diagnostic-implicit-fallthrough and
fixes failing locations. Since Impala uses C++17, this uses
C++17's [[fallthrough]] to indicate an explicit fallthrough.
This also adjusts clang-tidy's output to suggest [[fallthrough]]
as the preferred way to indicate fallthrough.
Testing:
- Ran core job
- Ran clang tidy
Change-Id: I6d65c92b442fa0317c3af228997571e124a54092
Reviewed-on: http://gerrit.cloudera.org:8080/20847
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Zihao Ye <ey...@163.com>
Reviewed-by: Michael Smith <mi...@cloudera.com>
---
M .clang-tidy
M be/src/benchmarks/atoi-benchmark.cc
M be/src/codegen/codegen-anyval-read-write-info.cc
M be/src/codegen/codegen-anyval.cc
M be/src/exec/orc/hdfs-orc-scanner.cc
M be/src/exec/parquet/parquet-column-stats.cc
M be/src/exprs/agg-fn-evaluator.cc
M be/src/exprs/anyval-util.h
M be/src/runtime/datetime-parser-common.cc
M be/src/runtime/descriptors.cc
M be/src/runtime/io/disk-file.cc
M be/src/runtime/raw-value.cc
M be/src/runtime/raw-value.inline.h
M be/src/runtime/runtime-filter-ir.cc
M be/src/runtime/types.cc
M be/src/service/query-result-set.cc
M be/src/util/bit-packing.inline.h
M be/src/util/hash-util.h
M be/src/util/jwt-util.cc
M be/src/util/redactor.cc
M be/src/util/string-parser.h
M bin/run_clang_tidy.sh
22 files changed, 105 insertions(+), 32 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Zihao Ye: Looks good to me, but someone else must approve
Michael Smith: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/20847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6d65c92b442fa0317c3af228997571e124a54092
Gerrit-Change-Number: 20847
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>