aaron.ballman added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:118
static void removeElseAndBrackets(DiagnosticBuilder &Diag, ASTContext &Context,
- const Stmt *Else, SourceLocation ElseLoc) {
+ const Stmt *Else, SourceLocation ElseLoc) {
auto Remap = [&](SourceLocation Loc) {
----------------
Unrelated formatting change?
================
Comment at:
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:189
+static bool hasPreprocessorBranchEndBetweenLocations(
+ ElseAfterReturnCheck::ConditionalBranchMap ConditionalBranchMap,
+ const SourceManager &SM, SourceLocation StartLoc, SourceLocation EndLoc) {
----------------
Should the map be passed by const ref to avoid copies?
================
Comment at:
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:203
+
+ auto &ConditionalBranches =
+ ConditionalBranchMap[SM.getFileID(ExpandedEndLoc)];
----------------
Should this be `const auto &` to be clear there are no modifications to the map?
================
Comment at:
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:214
+
+ // First conditional block that ends after ExpandedStartLoc
+ const auto *Begin =
----------------
Missing a full stop at the end of the comment.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:267
+
+ // These cases, Same as above but with an #else block
+ if (true) {
----------------
Same -> same
Also, missing a full stop.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:285
+
+// ensure it can handle macros
+#define RETURN return
----------------
ensure -> Ensure
Also, missing a full stop.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:304
+
+ // Whole statement is in a conditional block so diagnost as normal.
+#if 1
----------------
diagnost -> diagnose
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:313
+#endif
+}
----------------
We should probably add some tests for more pathological cases, like:
```
#if 1
if (true) {
return;
#else
{
#endif
} else {
return;
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91485/new/
https://reviews.llvm.org/D91485
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
