dblaikie added inline comments.
================ Comment at: clang/lib/Frontend/FrontendAction.cpp:553-565 + bool HasBegunSourceFile = false; + std::function<bool()> Operation = [&]() { + return BeginSourceFileImpl(CI, RealInput, HasBegunSourceFile); + }; + std::function<void()> Cleanup = [&]() { + if (HasBegunSourceFile) + CI.getDiagnosticClient().EndSourceFile(); ---------------- I guess this would be written as: ``` bool Success = BeginSourceFileImpl(CI, RealInput, HasBegunSourceFile)) if (!Success) { if (HasBegunSourceFile) CI.getDiagnosticClient().EndSourceFile(); CI.clearOutputFiles(/*EraseFiles=*/true); CI.getLangOpts().setCompilingModule(LangOptions::CMK_None); setCurrentInput(FrontendInputFile()); setCompilerInstance(nullptr); } return Success; ``` Which seems pretty legible - makes me unsure it's worth the complexity of building something like `doWithCleanup`, not that it's heinously complicated by any means - but all the traits stuff, etc. & ideally/in general this'd usually be dealt with with some kind of RAII ownership model rather than such a strong stateful transition of an existing object that needs to be undone, rather than destroying an object. ================ Comment at: llvm/include/llvm/Support/Cleanup.h:56-59 +template <typename ErrT> +ErrT doWithCleanup( + std::function<ErrT()> Fn, std::function<void()> CleanupFn, + std::function<bool(ErrT &)> IsFailure = &detail::isFailure<ErrT>) { ---------------- If this is going to be a template anyway, might as well use raw/generic/templated functor arguments and avoid the dynamic dispatch overhead of std::function? (or at least use llvm::function_ref) ================ Comment at: llvm/unittests/Support/CleanupTests.cpp:19 + bool Result = + doWithCleanup<bool>([] { return true; }, [&] { CleanupRun = true; }); + EXPECT_TRUE(Result); ---------------- Presumably doWithCleanup doesn't need to be called with an explicit template type argument most of the time/ever? It's probably good to test it in that way too, without passing the explicit template type argument. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110278/new/ https://reviews.llvm.org/D110278 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits