[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