[clang] [Clang][Sema] Add -Wredundant-defer to diagnose redundant `_Defer`s (PR #225748)

[Ambrose Leeb via cfe-commits]

================
@@ -997,6 +1014,14 @@ StmtResult Sema::ActOnIfStmt(SourceLocation IfLoc,
   if (!ConstevalOrNegatedConsteval && !elseStmt)
     DiagnoseEmptyStmtBody(RParenLoc, thenStmt, diag::warn_empty_if_body);
 
+  if (isa<DeferStmt>(thenStmt))
+    Diag(thenStmt->getBeginLoc(), diag::warn_redundant_defer)
+        << thenStmt->getSourceRange();
+
+  if (elseStmt && isa<DeferStmt>(elseStmt))
+    Diag(elseStmt->getBeginLoc(), diag::warn_redundant_defer)
+        << elseStmt->getSourceRange();
+
----------------
Sirraide wrote:

Ok, this looks like there’s some refactoring in order here because this entire situation here is a bit of a mess in terms of code duplication:

- The `CommaVisitor` check is repeated in `ActOnForStmt()` and `ActOnWhileStmt()` and I wouldn’t be surprised if it was also in the for-range code.
- This new check you’re adding is likewise repeated in those places.
- `DiagnoseEmptyStmtBody()` is done here directly, but _for some reason_, we don’t do that for loops and instead set `HasEmptyLoopBody` and then diagnose loops in `ActOnCompoundStatement`

I feel like we should introduce a function along the lines of `ActOnBodyStmt()` (there’s probably a better name for this) that does all (or at least some) of these things instead rather than duplicating that across every statement that has a statement as a body: `defer`, `if` (for both the then and else branches), `for`, `while`, range-`for`, `template for`; probably not `switch` though because `switch` is weird.

https://github.com/llvm/llvm-project/pull/225748