[PATCH] D29643: [analyzer] Do not duplicate call graph nodes for function that has definition and forward declaration.
This revision was automatically updated to reflect the committed changes. Closed by commit rL295644: [analyzer] Do not duplicate call graph nodes for functions that have definition… (authored by a.sidorin). Changed prior to commit: https://reviews.llvm.org/D29643?vs=87388=89089#toc Repository: rL LLVM https://reviews.llvm.org/D29643 Files: cfe/trunk/include/clang/Analysis/CallGraph.h cfe/trunk/test/Analysis/debug-CallGraph.c Index: cfe/trunk/include/clang/Analysis/CallGraph.h === --- cfe/trunk/include/clang/Analysis/CallGraph.h +++ cfe/trunk/include/clang/Analysis/CallGraph.h @@ -98,7 +98,7 @@ bool VisitFunctionDecl(FunctionDecl *FD) { // We skip function template definitions, as their semantics is // only determined when they are instantiated. -if (includeInGraph(FD)) { +if (includeInGraph(FD) && FD->isThisDeclarationADefinition()) { // Add all blocks declared inside this function to the graph. addNodesForBlocks(FD); // If this function has external linkage, anything could call it. Index: cfe/trunk/test/Analysis/debug-CallGraph.c === --- cfe/trunk/test/Analysis/debug-CallGraph.c +++ cfe/trunk/test/Analysis/debug-CallGraph.c @@ -43,8 +43,18 @@ void eee() {} void fff() { eee(); } +// This test case tests that forward declaration for the top-level function +// does not affect call graph construction. +void do_nothing() {} +void test_single_call(); +void test_single_call() { + do_nothing(); +} + // CHECK:--- Call graph Dump --- -// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ccc ddd eee fff $}} +// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ddd ccc eee fff do_nothing test_single_call $}} +// CHECK-NEXT: {{Function: test_single_call calls: do_nothing $}} +// CHECK-NEXT: {{Function: do_nothing calls: $}} // CHECK-NEXT: {{Function: fff calls: eee $}} // CHECK-NEXT: {{Function: eee calls: $}} // CHECK-NEXT: {{Function: ddd calls: ccc $}} Index: cfe/trunk/include/clang/Analysis/CallGraph.h === --- cfe/trunk/include/clang/Analysis/CallGraph.h +++ cfe/trunk/include/clang/Analysis/CallGraph.h @@ -98,7 +98,7 @@ bool VisitFunctionDecl(FunctionDecl *FD) { // We skip function template definitions, as their semantics is // only determined when they are instantiated. -if (includeInGraph(FD)) { +if (includeInGraph(FD) && FD->isThisDeclarationADefinition()) { // Add all blocks declared inside this function to the graph. addNodesForBlocks(FD); // If this function has external linkage, anything could call it. Index: cfe/trunk/test/Analysis/debug-CallGraph.c === --- cfe/trunk/test/Analysis/debug-CallGraph.c +++ cfe/trunk/test/Analysis/debug-CallGraph.c @@ -43,8 +43,18 @@ void eee() {} void fff() { eee(); } +// This test case tests that forward declaration for the top-level function +// does not affect call graph construction. +void do_nothing() {} +void test_single_call(); +void test_single_call() { + do_nothing(); +} + // CHECK:--- Call graph Dump --- -// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ccc ddd eee fff $}} +// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ddd ccc eee fff do_nothing test_single_call $}} +// CHECK-NEXT: {{Function: test_single_call calls: do_nothing $}} +// CHECK-NEXT: {{Function: do_nothing calls: $}} // CHECK-NEXT: {{Function: fff calls: eee $}} // CHECK-NEXT: {{Function: eee calls: $}} // CHECK-NEXT: {{Function: ddd calls: ccc $}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29643: [analyzer] Do not duplicate call graph nodes for function that has definition and forward declaration.
a.sidorin added a comment. Anna, I will commit. Thank you! https://reviews.llvm.org/D29643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29643: [analyzer] Do not duplicate call graph nodes for function that has definition and forward declaration.
zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Looks good. Thank you for catching this! Do you have commit access or should I commit on your behalf? https://reviews.llvm.org/D29643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29643: [analyzer] Do not duplicate call graph nodes for function that has definition and forward declaration.
IvanSidorenko created this revision. Fix in call graph construction: don't build call graph node for callee function twice for functions with forward declarations. Maybe the same fix should be done and for VisitObjCMethodDecl. Unfortunately, I have not enough expertise in ObjC, so I did not touch this code. Test case: void do_nothing() {} void test_single_call(); void test_single_call() { do_nothing(); } Output for the test case WITHOUT patch: Function: test_single_call calls: **do_nothing do_nothing** Function: do_nothing calls: Output for the test case WITH patch: Function: test_single_call calls: **do_nothing** Function: do_nothing calls: https://reviews.llvm.org/D29643 Files: include/clang/Analysis/CallGraph.h test/Analysis/debug-CallGraph.c Index: test/Analysis/debug-CallGraph.c === --- test/Analysis/debug-CallGraph.c +++ test/Analysis/debug-CallGraph.c @@ -43,8 +43,18 @@ void eee() {} void fff() { eee(); } +// This test case tests that forward declaration for the top-level function +// does not affect call graph construction. +void do_nothing() {} +void test_single_call(); +void test_single_call() { + do_nothing(); +} + // CHECK:--- Call graph Dump --- -// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ccc ddd eee fff $}} +// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ddd ccc eee fff do_nothing test_single_call $}} +// CHECK-NEXT: {{Function: test_single_call calls: do_nothing $}} +// CHECK-NEXT: {{Function: do_nothing calls: $}} // CHECK-NEXT: {{Function: fff calls: eee $}} // CHECK-NEXT: {{Function: eee calls: $}} // CHECK-NEXT: {{Function: ddd calls: ccc $}} Index: include/clang/Analysis/CallGraph.h === --- include/clang/Analysis/CallGraph.h +++ include/clang/Analysis/CallGraph.h @@ -98,7 +98,7 @@ bool VisitFunctionDecl(FunctionDecl *FD) { // We skip function template definitions, as their semantics is // only determined when they are instantiated. -if (includeInGraph(FD)) { +if (includeInGraph(FD) && FD->isThisDeclarationADefinition()) { // Add all blocks declared inside this function to the graph. addNodesForBlocks(FD); // If this function has external linkage, anything could call it. Index: test/Analysis/debug-CallGraph.c === --- test/Analysis/debug-CallGraph.c +++ test/Analysis/debug-CallGraph.c @@ -43,8 +43,18 @@ void eee() {} void fff() { eee(); } +// This test case tests that forward declaration for the top-level function +// does not affect call graph construction. +void do_nothing() {} +void test_single_call(); +void test_single_call() { + do_nothing(); +} + // CHECK:--- Call graph Dump --- -// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ccc ddd eee fff $}} +// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ddd ccc eee fff do_nothing test_single_call $}} +// CHECK-NEXT: {{Function: test_single_call calls: do_nothing $}} +// CHECK-NEXT: {{Function: do_nothing calls: $}} // CHECK-NEXT: {{Function: fff calls: eee $}} // CHECK-NEXT: {{Function: eee calls: $}} // CHECK-NEXT: {{Function: ddd calls: ccc $}} Index: include/clang/Analysis/CallGraph.h === --- include/clang/Analysis/CallGraph.h +++ include/clang/Analysis/CallGraph.h @@ -98,7 +98,7 @@ bool VisitFunctionDecl(FunctionDecl *FD) { // We skip function template definitions, as their semantics is // only determined when they are instantiated. -if (includeInGraph(FD)) { +if (includeInGraph(FD) && FD->isThisDeclarationADefinition()) { // Add all blocks declared inside this function to the graph. addNodesForBlocks(FD); // If this function has external linkage, anything could call it. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits