[PATCH] D33844: [clang-tidy] terminating continue check

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang-tidy/misc/TerminatingContinueCheck.cpp:42
+
+  auto Diag = diag(ContStmt->getLocStart(), "terminating 'continue'");
+  Diag << FixItHint::CreateReplacement(ContStmt->getSourceRange(), "break");
----------------
It was not clear to me what a "terminating 'continue'" was, just from seeing this error message. Wouldn't it make more sense to emit a self-explanatory diagnostic, such as

    'continue', in loop with false condition, is equivalent to 'break'

? And then you could even suggest a fixit of replacing 'continue' with 'break'... oh, which you already do. :)


================
Comment at: test/clang-tidy/misc-terminating-continue.cpp:7
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: terminating 'continue' [misc-terminating-continue]
+  } while(false);
+
----------------
I initially expected to see tests also for

    goto L1;
    while (false) {
        L1:
        continue;  // 'continue' is equivalent to 'break'
    }

    int v = 0;
    goto L1;
    for (; false; ++v) {
        L1:
        continue;  // 'continue' is NOT equivalent to 'break'
    }

although I assume your current patch doesn't actually diagnose the former, and that's probably fine.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D33844