[PATCH] D74214: [clang-tidy] Fix PR#34798 'readability-braces-around-statements breaks statements containing braces.'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp:76-77
+
+  // We need to check that it is not 'InitListExpr' which ends with 
+  // the tokens '};' because it will break the following analysis
+  tok::TokenKind NextTokKind;
----------------
ymandel wrote:
> ymandel wrote:
> > aaron.ballman wrote:
> > > Is there evidence that this behavior is desired? I have a hunch that this is a bug in Clang -- not all `InitListExpr`s will terminate with a semicolon, such as ones that appear as function arguments, like `foo({1, 2, 3});`, so I'm surprised to see it included here.
> > On a related note, are you sure the cause of this issue is `makeFileCharRange`? AFAIK, that does not involve any examination of tokens. It purely (attempts) to map from potentially a mix of expanded locations and file locations to purely file locations (and in the same file for that matter).
> I believe the issue lies with `DeclStmt`, not `InitListExpr`. Specifically, the source range provided by `DeclStmt`.  See https://godbolt.org/z/vVtQZ8. The non-decl statements have an end location on the token before the semi, whereas the decl statements given their end location as the semi.
Ah, yeah, that sounds plausible too (the end location should be before the semi in both places). Either way, I think we should be fixing this in the frontend rather than trying to hack around it in clang-tidy checks, if at all possible. This may require undoing hacks done elsewhere, though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74214/new/

https://reviews.llvm.org/D74214