IIRC, constructors are never inlined for 'new' right now because we never got 
the right region, and I was always hoping we'd implement proper allocator 
support first (http://llvm.org/bugs/show_bug.cgi?id=12014). Karthik Bhat was 
working on this but I don't remember how far he got. We also didn't have 
destructor support for 'delete' until last fall (also thanks to Karthik).

Other than that this is looking pretty good. More tests would be nice, and 
possibly a test on a real code base that would otherwise have false positives. 
Unfortunately the only one I can think of is LLVM itself, but even that could 
tell us how many false positives we've gone down by.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:768
@@ +767,3 @@
+
+  const CXXConstructExpr* ConstructE = NE->getConstructExpr();
+  if (!ConstructE)
----------------
Style nitpick: * next to the variable name.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:772-773
@@ +771,4 @@
+
+  QualType ConstructedTy = NE->getAllocatedType().getCanonicalType();
+  if (!ConstructedTy->getAsCXXRecordDecl())
+    return false;
----------------
getAsCXXRecordDecl will call getCanonicalType for you, so you can skip that 
step.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:776
@@ +775,3 @@
+
+  CXXConstructorDecl* CtorD = ConstructE->getConstructor();
+
----------------
Please make this const (and move the *).

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:785-786
@@ +784,4 @@
+
+    CtorParamPointeeT = getDeepPointeeType(CtorParamPointeeT).
+                        getCanonicalType().getUnqualifiedType();
+
----------------
I don't think you need to worry about either canonicalizing the type or 
stripping qualifiers.

http://reviews.llvm.org/D4025



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to