<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>Hi Jordan,</div><div><br></div><div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000"> // Was the usage ::new, i.e. is the global new to be used?</font></div><div><font class="Apple-style-span" color="#000000"> bool GlobalNew : 1;</font></div><div><font class="Apple-style-span" color="#000000">- // Do we allocate an array? If so, the first SubExpr is the size expression.</font></div><div><font class="Apple-style-span" color="#000000">- bool Array : 1;</font></div><div><font class="Apple-style-span" color="#000000"> // If this is an array allocation, does the usual deallocation</font></div><div><font class="Apple-style-span" color="#000000"> // function for the allocated type want to know the allocated size?</font></div><div><font class="Apple-style-span" color="#000000"> bool UsualArrayDeleteWantsSize : 1;</font></div><div><font class="Apple-style-span" color="#000000">- // The number of placement new arguments.</font></div><div><font class="Apple-style-span" color="#000000">- unsigned NumPlacementArgs : 13;</font></div></blockquote></div><div><div><br></div></div><div>With the number of bits needed by flags down this much, is it now feasible to sink this into the shared bitfield of Stmt?</div><div><br></div><div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000">+ // FIXME: Should these be removed?</font></div><div><font class="Apple-style-span" color="#000000"> typedef ExprIterator arg_iterator;</font></div><div><font class="Apple-style-span" color="#000000"> typedef ConstExprIterator const_arg_iterator;</font></div></blockquote></div><div><div><br></div></div><div>Really depends on who uses them and what usage patterns these places have. Does it make sense for them to iterate over the placement-new arguments?</div><div><br></div><div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000">--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h</font></div><div><font class="Apple-style-span" color="#000000">+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h</font></div><div><font class="Apple-style-span" color="#000000">@@ -196,10 +196,18 @@ public:</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> /// Check if the callee is declared in the system header.</font></div><div><font class="Apple-style-span" color="#000000"> bool isInSystemHeader() const {</font></div><div><font class="Apple-style-span" color="#000000">- if (const Decl *FD = getDecl()) {</font></div><div><font class="Apple-style-span" color="#000000">- const SourceManager &SM =</font></div><div><font class="Apple-style-span" color="#000000">- State->getStateManager().getContext().getSourceManager();</font></div><div><font class="Apple-style-span" color="#000000">- return SM.isInSystemHeader(FD->getLocation());</font></div><div><font class="Apple-style-span" color="#000000">+ if (const Decl *D = getDecl()) {</font></div><div><font class="Apple-style-span" color="#000000">+ SourceLocation Loc = D->getLocation();</font></div><div><font class="Apple-style-span" color="#000000">+ if (Loc.isValid()) {</font></div><div><font class="Apple-style-span" color="#000000">+ const SourceManager &SM =</font></div><div><font class="Apple-style-span" color="#000000">+ State->getStateManager().getContext().getSourceManager();</font></div><div><font class="Apple-style-span" color="#000000">+ return SM.isInSystemHeader(Loc); </font></div><div><font class="Apple-style-span" color="#000000">+ } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {</font></div><div><font class="Apple-style-span" color="#000000">+ // Operator new and operator delete may be implicitly declared.</font></div><div><font class="Apple-style-span" color="#000000">+ // We can't just check /any/ implicit declarations, though, because</font></div><div><font class="Apple-style-span" color="#000000">+ // C89 allows any function call to create an implicit declaration.</font></div><div><font class="Apple-style-span" color="#000000">+ return FD->isOverloadedOperator() && FD->isImplicit();</font></div><div><font class="Apple-style-span" color="#000000">+ }</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> return false;</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote></div><div><div><br></div></div><div>This looks unrelated.</div><div><br></div><div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000">+ // FIXME: Can the array size ever throw?</font></div><div><font class="Apple-style-span" color="#000000">+ if (const Expr *ArraySize = cast<CXXNewExpr>(this)->getArraySize())</font></div><div><font class="Apple-style-span" color="#000000">+ CT = MergeCanThrow(CT, ArraySize->CanThrow(C));</font></div></blockquote></div><div><div><br></div></div><div>Of course it can throw.</div><div>new int[callSomeFunction()]</div><div><br></div><div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000">+ } else if (Kind == OO_New || Kind == OO_Array_New) {</font></div><div><font class="Apple-style-span" color="#000000">+ // FIXME: Should the array brackets/array size be included for new[]?</font></div><div><font class="Apple-style-span" color="#000000">+ if (getRParenLoc().isValid())</font></div><div><font class="Apple-style-span" color="#000000">+ // There are explicit placement args.</font></div><div><font class="Apple-style-span" color="#000000">+ return SourceRange(getArg(0)->getSourceRange().getBegin(), getRParenLoc());</font></div><div><font class="Apple-style-span" color="#000000">+ else</font></div><div><font class="Apple-style-span" color="#000000">+ // There are no explicit placement args.</font></div><div><font class="Apple-style-span" color="#000000">+ return getArg(0)->getSourceRange();</font></div></blockquote></div><div><div><br></div></div><div>I think it would be better, but perhaps inconsistent with non-array new, since I don't think we include the type range there, do we?</div><div><br></div><div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000">--- a/lib/AST/StmtPrinter.cpp</font></div><div><font class="Apple-style-span" color="#000000">+++ b/lib/AST/StmtPrinter.cpp</font></div><div><font class="Apple-style-span" color="#000000">@@ -1154,6 +1154,8 @@ void StmtPrinter::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *Node) {</font></div><div><font class="Apple-style-span" color="#000000"> OS << '[';</font></div><div><font class="Apple-style-span" color="#000000"> PrintExpr(Node->getArg(1));</font></div><div><font class="Apple-style-span" color="#000000"> OS << ']';</font></div><div><font class="Apple-style-span" color="#000000">+ } else if (Kind == OO_New || Kind == OO_Array_New) {</font></div><div><font class="Apple-style-span" color="#000000">+ llvm_unreachable("should be handled within CXXNewExpr");</font></div><div><font class="Apple-style-span" color="#000000"> } else if (Node->getNumArgs() == 1) {</font></div><div><font class="Apple-style-span" color="#000000"> OS << OpStrings[Kind] << ' ';</font></div><div><font class="Apple-style-span" color="#000000"> PrintExpr(Node->getArg(0));</font></div><div><font class="Apple-style-span" color="#000000">@@ -1407,13 +1409,17 @@ void StmtPrinter::VisitCXXNewExpr(CXXNewExpr *E) {</font></div><div><font class="Apple-style-span" color="#000000"> if (E->isGlobalNew())</font></div><div><font class="Apple-style-span" color="#000000"> OS << "::";</font></div><div><font class="Apple-style-span" color="#000000"> OS << "new ";</font></div><div><font class="Apple-style-span" color="#000000">- unsigned NumPlace = E->getNumPlacementArgs();</font></div><div><font class="Apple-style-span" color="#000000">- if (NumPlace > 0) {</font></div><div><font class="Apple-style-span" color="#000000">+ if (E->getNumPlacementArgs() > 0) {</font></div><div><font class="Apple-style-span" color="#000000"> OS << "(";</font></div><div><font class="Apple-style-span" color="#000000">- PrintExpr(E->getPlacementArg(0));</font></div><div><font class="Apple-style-span" color="#000000">- for (unsigned i = 1; i < NumPlace; ++i) {</font></div><div><font class="Apple-style-span" color="#000000">- OS << ", ";</font></div><div><font class="Apple-style-span" color="#000000">- PrintExpr(E->getPlacementArg(i));</font></div><div><font class="Apple-style-span" color="#000000">+ bool NeedComma = false;</font></div><div><font class="Apple-style-span" color="#000000">+ for (CXXNewExpr::arg_iterator P = E->placement_arg_begin(),</font></div><div><font class="Apple-style-span" color="#000000">+ PEnd = E->placement_arg_end();</font></div><div><font class="Apple-style-span" color="#000000">+ P != PEnd; ++P) {</font></div><div><font class="Apple-style-span" color="#000000">+ if (NeedComma)</font></div><div><font class="Apple-style-span" color="#000000">+ OS << ", ";</font></div><div><font class="Apple-style-span" color="#000000">+ else</font></div><div><font class="Apple-style-span" color="#000000">+ NeedComma = true;</font></div><div><font class="Apple-style-span" color="#000000">+ PrintExpr(*P);</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> OS << ") ";</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote></div><div><div><br></div></div><div>I think it would make more sense to leave the printing of the new and the placement arguments to the CXXOperatorCallExpr. But I'm not very sure about this opinion.</div><div><br></div><div>It appears that there are quite a few places that get more complicated with this, e.g. the need to represent the allocation size expression in the AST. We need to find a way to improve this.</div><div><br></div><div>Sebastian</div></div></body></html>