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 accordingly. ================ Comment at: clang/lib/Analysis/CFG.cpp:1887-1889 + bool IsCXXRecordType = (Ty->getAsCXXRecordDecl() != nullptr); + if (IsCXXRecordType && + Ty->getAsCXXRecordDecl()->isAnyDestructorNoReturn()) ---------------- Here I'd suggest to deduplicate `Ty->getAsCXXRecordDecl()`. Implicit conversion of pointers to bool is idiomatic in LLVM. ================ Comment at: clang/lib/Analysis/CFG.cpp:5307-5308 + case CFGElement::CleanupFunction: + llvm_unreachable("getDestructorDecl should only be used with " + "ImplicitDtors"); case CFGElement::AutomaticObjectDtor: { ---------------- The unindent doesn't look right to me. ================ Comment at: clang/lib/Analysis/CFG.cpp:5850 + case CFGElement::Kind::CleanupFunction: { + OS << "CleanupFunction (" ---------------- Braces shouldn't be needed if you don't declare any variables. ================ Comment at: clang/lib/Analysis/ThreadSafety.cpp:2429-2438 + + case CFGElement::CleanupFunction: { + const CFGCleanupFunction &CF = BI.castAs<CFGCleanupFunction>(); + + LocksetBuilder.handleCall(nullptr, CF.getFunctionDecl(), + SxBuilder.createVariable(CF.getVarDecl()), + CF.getVarDecl()->getLocation()); ---------------- Should this be part of a follow-up? (For which you might revive D152504.) ================ Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1428-1429 +// CHECK-NEXT: 4: CFGScopeEnd(i) +void cleanup_int(int *i) { +} + ---------------- For our purposes, a pure declaration might be enough. ================ Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1433 + int i __attribute__((cleanup(cleanup_int))); +} ---------------- Ideas for more tests (apart from imitating destructor tests): * A variable in a block, so that more statements run before the function returns. * A function with multiple return paths, each of which has to run the cleanup. 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-bin/mailman/listinfo/cfe-commits