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

[Florian Gross via Phabricator via cfe-commits]

fgross updated this revision to Diff 99576.
fgross added a comment.

After some digging into it, here is my uneducated guess:

The comment in `findEndLocation` states that //"Loc points to the beginning of 
the last token before ';'"//. But `checkStmt` calls it with

`FileRange.getEnd().getLocWithOffset(-1)`

so in fact it points to the last char of the last token. For a string literal 
this would be '"' or ''', not enough for `Lexer::getLocForEndOfToken` to query 
the correct token type. It ends up moving behind the following ';' and skipping 
all whitespaces to next token.

I've updated the diff, this seems to resolve the issue. But I'm sure there is a 
way to pass the correct location in the first place.


https://reviews.llvm.org/D33354

Files:
  clang-tidy/readability/BracesAroundStatementsCheck.cpp


Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp
===================================================================
--- clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -54,14 +54,15 @@
 SourceLocation findEndLocation(SourceLocation LastTokenLoc,
                                const SourceManager &SM,
                                const ASTContext *Context) {
-  SourceLocation Loc = LastTokenLoc;
+  SourceLocation Loc =
+      Lexer::GetBeginningOfToken(LastTokenLoc, SM, Context->getLangOpts());
   // Loc points to the beginning of the last (non-comment non-ws) token
   // before end or ';'.
   assert(Loc.isValid());
   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) {
     // 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;



Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp
===================================================================
--- clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -54,14 +54,15 @@
 SourceLocation findEndLocation(SourceLocation LastTokenLoc,
                                const SourceManager &SM,
                                const ASTContext *Context) {
-  SourceLocation Loc = LastTokenLoc;
+  SourceLocation Loc =
+      Lexer::GetBeginningOfToken(LastTokenLoc, SM, Context->getLangOpts());
   // Loc points to the beginning of the last (non-comment non-ws) token
   // before end or ';'.
   assert(Loc.isValid());
   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) {
     // 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;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits