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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 marked 7 inline comments as done.
jyu2 added inline comments.


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:296
+
+  if (ThrowType->isReferenceType())
+    ThrowType = ThrowType->castAs<ReferenceType>()
----------------
aaron.ballman wrote:
> If `ThrowType` can be null, there should be a null pointer check here. If it cannot be null, you should use `getTypePtr()` above instead of `getTypePtrOrNull()`.
Good catch.  Add code and test to handle this


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:312
+    isCaught = ThrowTypeAsRecordType->isDerivedFrom(CaughtAsRecordType);
+  return isCaught;
+}
----------------
aaron.ballman wrote:
> There's really no point to using a local variable for this. You can return `true` above and return `false` here.
Right.  Changed


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:315
+
+static bool isThrowBeCaughtByHandlers(const CXXThrowExpr *CE,
+                                      const CXXTryStmt *TryStmt) {
----------------
aaron.ballman wrote:
> `isThrowCaughtByHandlers` (drop the Be)
removed "Be"


https://reviews.llvm.org/D33333