[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals

[Florian Gross via Phabricator via cfe-commits]

fgross created this revision.
Herald added a subscriber: xazax.hun.

Single-line if statements cause a false positive when the last token in the conditional statement is a char constant:

  if (condition)
    return 'a';

For some reason `findEndLocation` seems to skips too many (vertical) whitespaces in this case. The same problem already occured with string literals (https://reviews.llvm.org/D25558), and was fixed by adding a special check for this very case. I just extended the condition to also include char constants. No idea what really causes the issue though.


https://reviews.llvm.org/D33354

Files:
  clang-tidy/readability/BracesAroundStatementsCheck.cpp
  include/clang/Basic/TokenKinds.h
  test/clang-tidy/readability-braces-around-statements-single-line.cpp


Index: include/clang/Basic/TokenKinds.h
===================================================================
--- include/clang/Basic/TokenKinds.h
+++ include/clang/Basic/TokenKinds.h
@@ -82,12 +82,17 @@
          K == tok::utf32_string_literal;
 }
 
+/// \brief Return true if this is a char constant token
+inline bool isCharConstant(TokenKind K) {
+  return K == tok::char_constant || K == tok::wide_char_constant ||
+         K == tok::utf8_char_constant || K == tok::utf16_char_constant ||
+         K == tok::utf32_char_constant;
+}
+
 /// \brief Return true if this is a "literal" kind, like a numeric
 /// constant, string, etc.
 inline bool isLiteral(TokenKind K) {
-  return K == tok::numeric_constant || K == tok::char_constant ||
-         K == tok::wide_char_constant || K == tok::utf8_char_constant ||
-         K == tok::utf16_char_constant || K == tok::utf32_char_constant ||
+  return K == tok::numeric_constant || isCharConstant(K) ||
          isStringLiteral(K) || K == tok::angle_string_literal;
 }
 
Index: test/clang-tidy/readability-braces-around-statements-single-line.cpp
===================================================================
--- test/clang-tidy/readability-braces-around-statements-single-line.cpp
+++ test/clang-tidy/readability-braces-around-statements-single-line.cpp
@@ -31,3 +31,21 @@
   // CHECK-FIXES: if (cond("if4") /*comment*/) {
   // CHECK-FIXES: }
 }
+
+const char *test2() {
+  if (cond("if1"))
+    return "string";
+
+
+}
+
+char test3() {
+  if (cond("if1"))
+    return 'a';
+
+
+  if (cond("if1"))
+    return (char)L'a';
+
+
+}
Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp
===================================================================
--- clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -61,7 +61,8 @@
   bool SkipEndWhitespaceAndComments = true;
   tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
   if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
-      TokKind == tok::r_brace || isStringLiteral(TokKind)) {
+      TokKind == tok::r_brace || isCharConstant(TokKind) ||
+      isStringLiteral(TokKind)) {
     // If we are at ";" or "}", we found the last token. We could use as well
     // `if (isa<NullStmt>(S))`, but it wouldn't work for nested statements.
     SkipEndWhitespaceAndComments = false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33354.99549.patch
Type: text/x-patch
Size: 2376 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170519/72ba2008/attachment.bin>