[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:304
+                      ->getUnqualifiedDesugaredType();
+  if (CaughtType == nullptr || CaughtType == ThrowType)
+    return true;
----------------
aaron.ballman wrote:
> The check for a null `CaughtType` can be hoisted above the if statements, which can then be simplified.
This can still be hoisted higher in the function.


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:297
+
+  if (ThrowType == nullptr)
+    return false;
----------------
nit: `!ThrowType`


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:299
+    return false;
+  if (ThrowType && ThrowType->isReferenceType())
+    ThrowType = ThrowType->castAs<ReferenceType>()
----------------
No need for testing `ThrowType` for null here.


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:305
+    return true;
+  if (CaughtType && CaughtType->isReferenceType())
+    CaughtType = CaughtType->castAs<ReferenceType>()
----------------
No need for testing `CaughtType` for null here.


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:332
+      continue;
+    const CXXThrowExpr *CE =
+        dyn_cast<CXXThrowExpr>(B.getAs<CFGStmt>()->getStmt());
----------------
`const auto *`


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:341
+        continue;
+      if (const CXXTryStmt *Terminator =
+              dyn_cast_or_null<CXXTryStmt>(I->getTerminator()))
----------------
Oops, I should have suggested `const auto *` here with my original comment (sorry about that).


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:393
+
+static void EmitDiagForCXXThrowInNonThrowingFunc(SourceLocation OpLoc, Sema &S,
+                                                 const FunctionDecl *FD) {
----------------
For consistency with other code in the compiler, can you move the `Sema` param to be the first parameter?


https://reviews.llvm.org/D33333