[jira] [Commented] (IMPALA-12668) Turn on Clang Tidy's clang-diagnostic-implicit-fallthrough check

["ASF subversion and git services (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IMPALA-12668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17803301#comment-17803301 ] 

ASF subversion and git services commented on IMPALA-12668:
----------------------------------------------------------

Commit dd8ddf77c39ac34457497e160aefe707b7324331 in impala's branch refs/heads/master from Joe McDonnell
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=dd8ddf77c ]

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>


> Turn on Clang Tidy's clang-diagnostic-implicit-fallthrough check
> ----------------------------------------------------------------
>
>                 Key: IMPALA-12668
>                 URL: https://issues.apache.org/jira/browse/IMPALA-12668
>             Project: IMPALA
>          Issue Type: Improvement
>          Components: Backend
>    Affects Versions: Impala 4.4.0
>            Reporter: Joe McDonnell
>            Priority: Major
>             Fix For: Impala 4.4.0
>
>
> Clang Tidy's clang-diagnostic-implicit-fallthrough check find locations where a switch's case falls through without an annotation. i.e. something like this:
> {noformat}
> switch (foo) {
>   case 'BAR':
>     statement1;
>   case 'BAZ':
>     statement2;
>     break;
> }{noformat}
> If foo == BAR, then it executes both statement1 and statement2. If that is intended, it should be explicit and C++17 introduces [[fallthrough]] as a way to indicate this:
> {noformat}
> switch (foo) {
>   case 'BAR':
>     statement1;
>     [[fallthrough]];
>   case 'BAZ':
>     statement2;
>     break;
> }{noformat}
> This check tends to find subtle issues with switch/case statements, and we've had bugs around this in the past. We should enable this Clang Tidy check.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscribe@impala.apache.org
For additional commands, e-mail: issues-all-help@impala.apache.org