[PATCH] D19586: Misleading Indentation check
Daniel Marjamäki via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 13 09:56:26 PST 2017
danielmarjamaki added inline comments.
================
Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:42
+ const Stmt *Inside = nullptr;
+
+ if (const auto *CurrentIf = dyn_cast<IfStmt>(CurrentStmt)) {
----------------
I would rename Inside to Inner. That would make InnerLoc more "consistent".
================
Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:48
+ Inside = CurrentFor->getBody();
+ } else if (const auto CurrentWhile = dyn_cast<WhileStmt>(CurrentStmt)) {
+ Inside = CurrentWhile->getBody();
----------------
nit: write "const auto *CurrentWhile...". missing "*"?
================
Comment at: test/clang-tidy/readability-misleading-indentation.cpp:21
+ foo2();
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: potential dangling 'else' [readability-misleading-indentation]
+
----------------
I am skeptic about this warning message.
Why does it say "potential". I would say that in this test case the indentation _is_ "dangling".
The message is not very clear to me. I personally don't intuitively understand what is wrong without looking at the code.
I don't know what it should say. Maybe:
```
different indentation for 'if' and 'else'
```
https://reviews.llvm.org/D19586
More information about the cfe-commits
mailing list