[PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.

================
Comment at: lib/CodeGen/CGCXX.cpp:44
@@ +43,3 @@
+  // destructors.
+  const ASTRecordLayout &Layout = Context.getASTRecordLayout(D->getParent());
+
----------------
This is unused.

================
Comment at: lib/CodeGen/CGCXX.cpp:47
@@ +46,3 @@
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+      Layout.getFieldCount() > 0 && HasTrivialField(*this, D))
+    return true;
----------------
This is unnecessary:
 Layout.getFieldCount() > 0

================
Comment at: lib/CodeGen/CGCXX.cpp:134
@@ +133,3 @@
+  //  an alias, unless this class owns no members.
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor)
+    return true;
----------------
This function is called from TryEmitBaseDestructorAsAlias.
This check is stronger than the one in the calling function, so that one has no effect.
Maybe move the check to this function?

================
Comment at: lib/CodeGen/CGClass.cpp:1355
@@ -1357,1 +1354,3 @@
+static bool CanSkipVTablePointerInitialization(CodeGenFunction &CGF,
+                                               ASTContext &Context,
                                                const CXXDestructorDecl *Dtor) {
----------------
Remove context from arguments, use CGF.getContext()
Consider making this a method of CodegenFunction


================
Comment at: lib/CodeGen/CGClass.cpp:1489
@@ -1541,2 +1488,3 @@
     llvm::Value *ShouldDeleteCondition;
+
   public:
----------------
extra line

================
Comment at: lib/CodeGen/CGClass.cpp:1500
@@ -1553,1 +1499,3 @@
+      llvm::Value *ShouldCallDelete =
+          CGF.Builder.CreateIsNull(ShouldDeleteCondition);
       CGF.Builder.CreateCondBr(ShouldCallDelete, continueBB, callDeleteBB);
----------------
unrelated changes

================
Comment at: lib/CodeGen/CGClass.cpp:1728
@@ -1660,1 +1727,3 @@
 
+  EHStack.pushCleanup<SanitizeDtor>(NormalCleanup, DD);
+
----------------
1. Only do this if sanitizing.
2. I think this needs to be NormalAndEHCleanup


http://reviews.llvm.org/D12022