beanz created this revision. beanz added reviewers: lhames, dblaikie. Herald added subscribers: dexonsmith, mgorny. beanz requested review of this revision. Herald added projects: clang, LLVM.
This patch adds a new `doWithCleanup` abstraction to LLVM that works with `llvm::Error`, `int`, `bool`, and <insert your own type here> return types that propagate status. This abstraction is a more general way of having a conditional cleanup that can be used with multiple return paths out of a function. Test cases and a first use case in clang are part of the patch. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D110278 Files: clang/include/clang/Frontend/FrontendAction.h clang/lib/Frontend/FrontendAction.cpp llvm/include/llvm/Support/Cleanup.h llvm/unittests/Support/CMakeLists.txt llvm/unittests/Support/CleanupTests.cpp
Index: llvm/unittests/Support/CleanupTests.cpp =================================================================== --- /dev/null +++ llvm/unittests/Support/CleanupTests.cpp @@ -0,0 +1,86 @@ +//===----- unittests/CleanupTests.cpp - Cleanup.h tests -------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "llvm/Support/Cleanup.h" +#include "llvm/Support/Error.h" + +#include "gtest/gtest.h" + +using namespace llvm; + +TEST(Error, DoWithCleanupBool) { + bool CleanupRun = false; + bool Result = + doWithCleanup<bool>([] { return true; }, [&] { CleanupRun = true; }); + EXPECT_TRUE(Result); + EXPECT_FALSE(CleanupRun); + + Result = + doWithCleanup<bool>([] { return false; }, [&] { CleanupRun = true; }); + EXPECT_FALSE(Result); + EXPECT_TRUE(CleanupRun); +} + +TEST(Error, DoWithCleanupError) { + bool CleanupRun = false; + llvm::consumeError(doWithCleanup<llvm::Error>( + [&] { return Error::success(); }, [&] { CleanupRun = true; })); + + EXPECT_FALSE(CleanupRun); + + llvm::consumeError(doWithCleanup<llvm::Error>( + [&] { return errorCodeToError(inconvertibleErrorCode()); }, + [&] { CleanupRun = true; })); + EXPECT_TRUE(CleanupRun); +} + +TEST(Error, DoWithCleanupInt) { + bool CleanupRun = false; + int Result = doWithCleanup<int>([] { return 0; }, [&] { CleanupRun = true; }); + EXPECT_EQ(Result, 0); + EXPECT_FALSE(CleanupRun); + + Result = doWithCleanup<int>([] { return 1; }, [&] { CleanupRun = true; }); + EXPECT_EQ(Result, 1); + EXPECT_TRUE(CleanupRun); +} + +enum class PetType { Doggo, Kitteh, Birb, WhoozieWhatsit }; + +TEST(Error, DoWithCleanupOverrideFailure) { + bool CleanupRun = false; + PetType Result = doWithCleanup<PetType>( + [] { return PetType::Doggo; }, [&] { CleanupRun = true; }, + [](PetType &E) { return E == PetType::WhoozieWhatsit; }); + EXPECT_EQ(Result, PetType::Doggo); + EXPECT_FALSE(CleanupRun); + + Result = doWithCleanup<PetType>( + [] { return PetType::WhoozieWhatsit; }, [&] { CleanupRun = true; }, + [](PetType &E) { return E == PetType::WhoozieWhatsit; }); + EXPECT_EQ(Result, PetType::WhoozieWhatsit); + EXPECT_TRUE(CleanupRun); +} + +enum class ResultTy { + Failure = 0, + Success, +}; + +TEST(Error, DoWithCleanupEnumNonZeroFailure) { + bool CleanupRun = false; + ResultTy Result = doWithCleanup<ResultTy>([] { return ResultTy::Success; }, + [&] { CleanupRun = true; }); + EXPECT_EQ(Result, ResultTy::Success); + EXPECT_FALSE(CleanupRun); + + Result = doWithCleanup<ResultTy>([] { return ResultTy::Failure; }, + [&] { CleanupRun = true; }); + EXPECT_EQ(Result, ResultTy::Failure); + EXPECT_TRUE(CleanupRun); +} Index: llvm/unittests/Support/CMakeLists.txt =================================================================== --- llvm/unittests/Support/CMakeLists.txt +++ llvm/unittests/Support/CMakeLists.txt @@ -18,6 +18,7 @@ Casting.cpp CheckedArithmeticTest.cpp Chrono.cpp + CleanupTests.cpp CommandLineTest.cpp CompressionTest.cpp ConvertUTFTest.cpp Index: llvm/include/llvm/Support/Cleanup.h =================================================================== --- /dev/null +++ llvm/include/llvm/Support/Cleanup.h @@ -0,0 +1,67 @@ +//===- llvm/Support/Cleanup.h - Cleanup on failure abstraction --*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines helper abstractions for cleaning up resources on failure. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_SUPPORT_CLEANUP_H +#define LLVM_SUPPORT_CLEANUP_H + +#include "llvm/Support/Error.h" + +#include <functional> +#include <type_traits> + +namespace llvm { + +/// Helper templates for determining if a result is a failure. +namespace detail { + +/// Evaluator for if an llvm::Error contains a failure. +template <typename ErrT> +typename std::enable_if<std::is_same<ErrT, llvm::Error>::value, bool>::type +isFailure(ErrT &Err) { + return (bool)Err; +} + +/// Evaluator for c-style int error returns where success is 0. +template <typename ErrT> +typename std::enable_if<std::is_same<ErrT, int>::value, bool>::type +isFailure(ErrT &Err) { + return 0 != Err; +} + +/// Evaluator for boolean error returns where success is true. +template <typename ErrT> +typename std::enable_if<!(std::is_same<ErrT, int>::value || + std::is_same<ErrT, llvm::Error>::value), + bool>::type +isFailure(ErrT &Err) { + return !(bool)Err; +} +} // namespace detail + +/// doWithCleanup abstraction performs an operation and cleanup on failure. +/// +/// @param Fn std::function to perform which returns the error type. +/// @param CleanupFn std::function to perform cleanup on failure result. +/// @param IsFailure (optional) std::function that takes a reference of the +/// error type, and returns true if the value is an error and false otherwise. +template <typename ErrT> +ErrT doWithCleanup( + std::function<ErrT()> Fn, std::function<void()> CleanupFn, + std::function<bool(ErrT &)> IsFailure = &detail::isFailure<ErrT>) { + ErrT Result = Fn(); + if (IsFailure(Result)) + CleanupFn(); + return Result; +} +} // namespace llvm + +#endif // LLVM_SUPPORT_CLEANUP_H Index: clang/lib/Frontend/FrontendAction.cpp =================================================================== --- clang/lib/Frontend/FrontendAction.cpp +++ clang/lib/Frontend/FrontendAction.cpp @@ -27,8 +27,8 @@ #include "clang/Serialization/ASTDeserializationListener.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/GlobalModuleIndex.h" -#include "llvm/ADT/ScopeExit.h" #include "llvm/Support/BuryPointer.h" +#include "llvm/Support/Cleanup.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -550,27 +550,33 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, const FrontendInputFile &RealInput) { + bool HasBegunSourceFile = false; + std::function<bool()> Operation = [&]() { + return BeginSourceFileImpl(CI, RealInput, HasBegunSourceFile); + }; + std::function<void()> Cleanup = [&]() { + if (HasBegunSourceFile) + CI.getDiagnosticClient().EndSourceFile(); + CI.clearOutputFiles(/*EraseFiles=*/true); + CI.getLangOpts().setCompilingModule(LangOptions::CMK_None); + setCurrentInput(FrontendInputFile()); + setCompilerInstance(nullptr); + }; + return llvm::doWithCleanup(Operation, Cleanup); +} + +bool FrontendAction::BeginSourceFileImpl(CompilerInstance &CI, + const FrontendInputFile &RealInput, + bool &HasBegunSourceFile) { FrontendInputFile Input(RealInput); assert(!Instance && "Already processing a source file!"); assert(!Input.isEmpty() && "Unexpected empty filename!"); setCurrentInput(Input); setCompilerInstance(&CI); - bool HasBegunSourceFile = false; bool ReplayASTFile = Input.getKind().getFormat() == InputKind::Precompiled && usesPreprocessorOnly(); - // If we fail, reset state since the client will not end up calling the - // matching EndSourceFile(). All paths that return true should release this. - auto FailureCleanup = llvm::make_scope_exit([&]() { - if (HasBegunSourceFile) - CI.getDiagnosticClient().EndSourceFile(); - CI.clearOutputFiles(/*EraseFiles=*/true); - CI.getLangOpts().setCompilingModule(LangOptions::CMK_None); - setCurrentInput(FrontendInputFile()); - setCompilerInstance(nullptr); - }); - if (!BeginInvocation(CI)) return false; @@ -689,7 +695,6 @@ if (!CI.hasASTConsumer()) return false; - FailureCleanup.release(); return true; } @@ -730,7 +735,6 @@ if (!CI.InitializeSourceManager(CurrentInput)) return false; - FailureCleanup.release(); return true; } @@ -942,7 +946,6 @@ CI.getASTContext().setExternalSource(Override); } - FailureCleanup.release(); return true; } Index: clang/include/clang/Frontend/FrontendAction.h =================================================================== --- clang/include/clang/Frontend/FrontendAction.h +++ clang/include/clang/Frontend/FrontendAction.h @@ -44,6 +44,9 @@ std::unique_ptr<ASTConsumer> CreateWrappedASTConsumer(CompilerInstance &CI, StringRef InFile); + bool BeginSourceFileImpl(CompilerInstance &CI, const FrontendInputFile &Input, + bool &HasBegunSourceFile); + protected: /// @name Implementation Action Interface /// @{
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
