[PATCH] D157385: [clang][CFG] Cleanup functions

This revision was automatically updated to reflect the committed changes. Closed by commit rGad4a51302777: [clang][CFG] Cleanup functions (authored by tbaeder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157385/new/ https://reviews.llvm.org/D1573

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder added inline comments. Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1472 +// CHECK-NEXT:2: (CXXConstructExpr, [B1.3], F) +// CHECK-NEXT:3: F f __attribute__((cleanup(cleanup_F))); +// CHECK-NEXT:4: CleanupFunction (cleanup_F) aaronpu

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder updated this revision to Diff 556836. tbaeder marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157385/new/ https://reviews.llvm.org/D157385 Files: clang/include/clang/Analysis/CFG.h clang/lib/Analysis/CFG.cpp clang/lib/Analysis/PathDiagnostic.

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added inline comments. Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1472 +// CHECK-NEXT:2: (CXXConstructExpr, [B1.3], F) +// CHECK-NEXT:3: F f __attribute__((cleanup(cleanup_F))); +// CHECK-NEXT:4: CleanupFunction (cleanup_F) Th

[PATCH] D157385: [clang][CFG] Cleanup functions

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. In D157385#4634591 , @tbaeder wrote: > @steakhal Double lifetime ends should be fixed now. I haven't verified, but it should be good now. CHANGE

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder marked an inline comment as done. tbaeder added a comment. @steakhal Double lifetime ends should be fixed now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157385/new/ https://reviews.llvm.org/D157385 ___ cfe-commits mailing list cfe

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder updated this revision to Diff 555403. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157385/new/ https://reviews.llvm.org/D157385 Files: clang/include/clang/Analysis/CFG.h clang/lib/Analysis/CFG.cpp clang/lib/Analysis/PathDiagnostic.cpp clang/lib/StaticAnalyzer/Core/ExprEn

[PATCH] D157385: [clang][CFG] Cleanup functions

steakhal added a comment. When I added `-analyzer-config cfg-lifetime=true` to `clang/test/Analysis/scopes-cfg-output.cpp`, suddenly duplicated lifetime ends entries appeared where we have `CleanupFunctions`. My output is: void test_cleanup_functions() [B2 (ENTRY)] Succs (1): B1

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:1901-1902 appendLifetimeEnds(Block, VD, S); -if (BuildOpts.AddImplicitDtors) +if (BuildOpts.AddImplicitDtors && !hasTrivialDestructor(VD)) appendAutomaticObjDtor(Block, VD, S); +if (Ha

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder added a comment. Not sure I have enough CFG knowledge. Do I just need to create another noreturn block for the cleanup function? This is the CFG I get when both the cleanup function and the destructor are noreturn: int main() [B4 (ENTRY)] Succs (1): B3 [B1] 1: CFGS

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder updated this revision to Diff 555251. tbaeder marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157385/new/ https://reviews.llvm.org/D157385 Files: clang/include/clang/Analysis/CFG.h clang/lib/Analysis/CFG.cpp clang/lib/Analysis/PathDiagnostic.

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder marked 2 inline comments as done. tbaeder added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2113 + +bool CFGBuilder::hasTrivialDestructor(VarDecl *VD) const { // Check for const references bound to temporary. Set type to pointee. steakhal w

[PATCH] D157385: [clang][CFG] Cleanup functions

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. This is a great improvement. When I saw that clang now supports it and e.g. the CSA operates on the CFG, I also considered adding this. Now I don't need to do it, so many thanks!

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added a comment. Looks good to me, but let's wait for the CFG maintainers to approve it. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2429 } + case CFGElement::TemporaryDtor: { Could you remove the added line? CHANGES SINCE L

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder updated this revision to Diff 552608. tbaeder marked 2 inline comments as done. Herald added subscribers: steakhal, martong. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157385/new/ https://reviews.llvm.org/D157385 Files: clang/include/clang/Analysis/CFG.h clang/lib/Analysis

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added inline comments. Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1480 +public: + ~F() {} +}; tbaeder wrote: > aaronpuchert wrote: > > As with the cleanup function, a definition shouldn't be necessary. > Is there a way to test whether the

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder marked an inline comment as done. tbaeder added inline comments. Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1473-1474 +// CHECK-NEXT:3: F f __attribute__((cleanup(cleanup_F))); +// CHECK-NEXT:4: [B1.3].~F() (Implicit destructor) +// CHECK-NEXT:5: Cl

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added inline comments. Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1473-1474 +// CHECK-NEXT:3: F f __attribute__((cleanup(cleanup_F))); +// CHECK-NEXT:4: [B1.3].~F() (Implicit destructor) +// CHECK-NEXT:5: CleanupFunction (cleanup_F) +// CHECK-N

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2429-2438 + +case CFGElement::CleanupFunction: { + const CFGCleanupFunction &CF = BI.castAs(); + + LocksetBuilder.handleCall(nullptr, CF.getFunctionDecl(), +

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder updated this revision to Diff 552018. tbaeder marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157385/new/ https://reviews.llvm.org/D157385 Files: clang/include/clang/Analysis/CFG.h clang/lib/Analysis/CFG.cpp clang/lib/Analysis/ThreadSafety.cp

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added a comment. For me this looks good, but I'd like @NoQ to sign off on it. Comment at: clang/lib/Analysis/CFG.cpp:1874 +if (needsAutomaticDestruction(D)) DeclsNonTrivial.push_back(D); I'm wondering if you should rename this accordin

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder updated this revision to Diff 551131. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157385/new/ https://reviews.llvm.org/D157385 Files: clang/include/clang/Analysis/CFG.h clang/lib/Analysis/CFG.cpp clang/lib/Analysis/ThreadSafety.cpp clang/test/Analysis/scopes-cfg-output.

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157385/new/ https://reviews.llvm.org/D157385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D157385: [clang][CFG] Cleanup functions

tbaeder created this revision. tbaeder added reviewers: aaronpuchert, NoQ, clang. Herald added a project: All. tbaeder requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This might be a little light on the testing side. Repository: rG LLVM