At 08:45 PM 12/22/2010, Ted Kremenek wrote:
Hi Jim,First, sorry for the delay in coming back this patch.If I understand you correctly, the assumption you are making here is that the default arguments will be evaluated once we do the function call. That means they are evaluated on the callee's side, not the caller's. Is this the right thing to do?As you know, checkers can monitor function/method calls at the the exact point before and after the call is evaluated. Technically this means that all the parameters, including the default parameters, have been evaluated. Also, technically the function call may never take place if one of the default arguments introduces control-flow that aborts the call. For example:void halt() __attribute__((noreturn)); void foo(int x = (halt(), 1)); void test() { foo(); // the function foo() never actually gets called. } If we compile this to LLVM bytecode, we get: define void @_Z4testv() ssp { call void @_Z4haltv() noreturn unreachable ; No predecessors! call void @_Z3fooi(i32 1) ret void }From the LLVM bytecode, we can see that technically the default arguments are evaluated in the caller, not the callee. This means if we are going to truly support default parameters we need to embed CFG support for them in the caller's CFG, not the callee's. I think the problem with your patch is it really skirts around the issue, and I'd rather we implement support for default parameters for real. From this design perspective, I also think this has nothing to do with interprocedural analysis. Intraprocedural analyses will need to be able to reason about default parameters as well.What do you think?
I agree. How about this revised patch which displays the actual default arguments when the AST is dumped and actually handles the argument itself?
Index: lib/AST/StmtDumper.cpp
===================================================================
--- lib/AST/StmtDumper.cpp (revision 122493)
+++ lib/AST/StmtDumper.cpp (working copy)
@@ -157,6 +157,7 @@
// C++
void VisitCXXNamedCastExpr(CXXNamedCastExpr *Node);
void VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *Node);
+ void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Node);
void VisitCXXThisExpr(CXXThisExpr *Node);
void VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr *Node);
void VisitCXXConstructExpr(CXXConstructExpr *Node);
@@ -500,6 +501,12 @@
OS << " " << (Node->getValue() ? "true" : "false");
}
+void StmtDumper::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Node) {
+ DumpExpr(Node);
+ OS << "\n";
+ DumpSubTree(Node->getExpr());
+}
+
void StmtDumper::VisitCXXThisExpr(CXXThisExpr *Node) {
DumpExpr(Node);
OS << " this";
Index: lib/EntoSA/Checkers/ExprEngine.cpp
===================================================================
--- lib/EntoSA/Checkers/ExprEngine.cpp (revision 122493)
+++ lib/EntoSA/Checkers/ExprEngine.cpp (working copy)
@@ -753,7 +753,6 @@
// C++ stuff we don't support yet.
case Stmt::CXXBindTemporaryExprClass:
case Stmt::CXXCatchStmtClass:
- case Stmt::CXXDefaultArgExprClass:
case Stmt::CXXDependentScopeMemberExprClass:
case Stmt::ExprWithCleanupsClass:
case Stmt::CXXNullPtrLiteralExprClass:
@@ -911,6 +910,12 @@
break;
}
+ case Stmt::CXXDefaultArgExprClass: {
+ const CXXDefaultArgExpr *DAE = cast<CXXDefaultArgExpr>(S);
+ Visit(DAE->getExpr(), Pred, Dst);
+ break;
+ }
+
case Stmt::CXXDeleteExprClass: {
const CXXDeleteExpr *CDE = cast<CXXDeleteExpr>(S);
defaultarg.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
