[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] D12301: [PATCH] New checker for UB in handler of a function-try-block
From:       Richard Smith via cfe-commits <cfe-commits () lists ! llvm ! org>
Date:       2015-08-31 22:13:36
Message-ID: 44d7c53caabab8f0a7d9e4c92b168503 () localhost ! localdomain
[Download RAW message or body]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Couple of thoughts, but LGTM.


================
Comment at: lib/Sema/SemaExprMember.cpp:889
@@ +888,3 @@
+    S = S->getParent();
+  } while (S != S->getFnParent());
+  return false;
----------------
Maybe convert from a do-while into a while loop (or even a for loop)? We might not be \
in a function at all (for instance, we might be in a default member initializer), and \
that would let us bail out earlier in that case.

================
Comment at: lib/Sema/SemaExprMember.cpp:969-972
@@ +968,6 @@
+  if (S && BaseExpr && isa<CXXThisExpr>(BaseExpr->IgnoreImpCasts()) &&
+      IsInFnTryBlockHandler(S)) {
+    const auto *FD = getCurFunctionDecl();
+    bool IsDestructor = isa<CXXDestructorDecl>(FD);
+    if (IsDestructor || isa<CXXConstructorDecl>(FD)) {
+      Diag(MemberLoc, diag::warn_cdtor_function_try_handler_mem_expr)
----------------
The `FD` checks are cheaper than the scope check; maybe reorder this to check the \
kind of the function first?


http://reviews.llvm.org/D12301



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]