https://github.com/vhscampos updated https://github.com/llvm/llvm-project/pull/116406
>From bfd7a4cd935c45b84d270b12d1989531d4522732 Mon Sep 17 00:00:00 2001 From: Victor Campos <victor.cam...@arm.com> Date: Fri, 15 Nov 2024 16:15:04 +0000 Subject: [PATCH] [clang][Driver] Use shared_ptr in the Compilation class This patch replaces uses of raw pointers by shared_ptrs in the Driver's Compilation class. The manual memory management which was done before this patch could be error prone. Plus, code is now simpler. --- clang/include/clang/Driver/Compilation.h | 11 +++--- clang/lib/Driver/Compilation.cpp | 44 +++++++++--------------- clang/lib/Driver/Driver.cpp | 6 ++-- 3 files changed, 25 insertions(+), 36 deletions(-) diff --git a/clang/include/clang/Driver/Compilation.h b/clang/include/clang/Driver/Compilation.h index 36ae85c4245143..db44d2bb68d693 100644 --- a/clang/include/clang/Driver/Compilation.h +++ b/clang/include/clang/Driver/Compilation.h @@ -61,11 +61,11 @@ class Compilation { OrderedOffloadingToolchains; /// The original (untranslated) input argument list. - llvm::opt::InputArgList *Args; + std::shared_ptr<llvm::opt::InputArgList> Args; /// The driver translated arguments. Note that toolchains may perform their /// own argument translation. - llvm::opt::DerivedArgList *TranslatedArgs; + std::shared_ptr<llvm::opt::DerivedArgList> TranslatedArgs; /// The list of actions we've created via MakeAction. This is not accessible /// to consumers; it's here just to manage ownership. @@ -100,7 +100,7 @@ class Compilation { return false; } }; - std::map<TCArgsKey, llvm::opt::DerivedArgList *> TCArgs; + std::map<TCArgsKey, std::shared_ptr<llvm::opt::DerivedArgList>> TCArgs; /// Temporary files which should be removed on exit. llvm::opt::ArgStringList TempFiles; @@ -134,8 +134,9 @@ class Compilation { public: Compilation(const Driver &D, const ToolChain &DefaultToolChain, - llvm::opt::InputArgList *Args, - llvm::opt::DerivedArgList *TranslatedArgs, bool ContainsError); + std::shared_ptr<llvm::opt::InputArgList> Args, + std::shared_ptr<llvm::opt::DerivedArgList> TranslatedArgs, + bool ContainsError); ~Compilation(); const Driver &getDriver() const { return TheDriver; } diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp index ad077d5bbfa69a..b8025d6a46f2e6 100644 --- a/clang/lib/Driver/Compilation.cpp +++ b/clang/lib/Driver/Compilation.cpp @@ -33,7 +33,8 @@ using namespace driver; using namespace llvm::opt; Compilation::Compilation(const Driver &D, const ToolChain &_DefaultToolChain, - InputArgList *_Args, DerivedArgList *_TranslatedArgs, + std::shared_ptr<InputArgList> _Args, + std::shared_ptr<DerivedArgList> _TranslatedArgs, bool ContainsError) : TheDriver(D), DefaultToolChain(_DefaultToolChain), Args(_Args), TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError) { @@ -47,14 +48,6 @@ Compilation::~Compilation() { // the file names might be derived from the input arguments. if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles) CleanupFileList(TempFiles); - - delete TranslatedArgs; - delete Args; - - // Free any derived arg lists. - for (auto Arg : TCArgs) - if (Arg.second != TranslatedArgs) - delete Arg.second; } const DerivedArgList & @@ -63,41 +56,39 @@ Compilation::getArgsForToolChain(const ToolChain *TC, StringRef BoundArch, if (!TC) TC = &DefaultToolChain; - DerivedArgList *&Entry = TCArgs[{TC, BoundArch, DeviceOffloadKind}]; + std::shared_ptr<DerivedArgList> &Entry = + TCArgs[{TC, BoundArch, DeviceOffloadKind}]; if (!Entry) { SmallVector<Arg *, 4> AllocatedArgs; - DerivedArgList *OpenMPArgs = nullptr; + std::shared_ptr<DerivedArgList> OpenMPArgs; // Translate OpenMP toolchain arguments provided via the -Xopenmp-target flags. if (DeviceOffloadKind == Action::OFK_OpenMP) { const ToolChain *HostTC = getSingleOffloadToolChain<Action::OFK_Host>(); bool SameTripleAsHost = (TC->getTriple() == HostTC->getTriple()); - OpenMPArgs = TC->TranslateOpenMPTargetArgs( - *TranslatedArgs, SameTripleAsHost, AllocatedArgs); + OpenMPArgs.reset(TC->TranslateOpenMPTargetArgs( + *TranslatedArgs, SameTripleAsHost, AllocatedArgs)); } - DerivedArgList *NewDAL = nullptr; + std::shared_ptr<DerivedArgList> NewDAL; if (!OpenMPArgs) { - NewDAL = TC->TranslateXarchArgs(*TranslatedArgs, BoundArch, - DeviceOffloadKind, &AllocatedArgs); + NewDAL.reset(TC->TranslateXarchArgs(*TranslatedArgs, BoundArch, + DeviceOffloadKind, &AllocatedArgs)); } else { - NewDAL = TC->TranslateXarchArgs(*OpenMPArgs, BoundArch, DeviceOffloadKind, - &AllocatedArgs); + NewDAL.reset(TC->TranslateXarchArgs(*OpenMPArgs, BoundArch, + DeviceOffloadKind, &AllocatedArgs)); if (!NewDAL) NewDAL = OpenMPArgs; - else - delete OpenMPArgs; } if (!NewDAL) { - Entry = TC->TranslateArgs(*TranslatedArgs, BoundArch, DeviceOffloadKind); + Entry.reset( + TC->TranslateArgs(*TranslatedArgs, BoundArch, DeviceOffloadKind)); if (!Entry) Entry = TranslatedArgs; } else { - Entry = TC->TranslateArgs(*NewDAL, BoundArch, DeviceOffloadKind); + Entry.reset(TC->TranslateArgs(*NewDAL, BoundArch, DeviceOffloadKind)); if (!Entry) - Entry = NewDAL; - else - delete NewDAL; + Entry = std::shared_ptr<DerivedArgList>(NewDAL); } // Add allocated arguments to the final DAL. @@ -290,9 +281,6 @@ void Compilation::initCompilationForDiagnostics() { // Force re-creation of the toolchain Args, otherwise our modifications just // above will have no effect. - for (auto Arg : TCArgs) - if (Arg.second != TranslatedArgs) - delete Arg.second; TCArgs.clear(); // Redirect stdout/stderr to /dev/null. diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 93e85f7dffe35a..b50bfe2d871914 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1485,7 +1485,7 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) { std::make_unique<InputArgList>(std::move(Args)); // Perform the default argument translations. - DerivedArgList *TranslatedArgs = TranslateInputArgs(*UArgs); + std::unique_ptr<DerivedArgList> TranslatedArgs(TranslateInputArgs(*UArgs)); // Owned by the host. const ToolChain &TC = getToolChain( @@ -1544,8 +1544,8 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) { } // The compilation takes ownership of Args. - Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs, - ContainsError); + Compilation *C = new Compilation(*this, TC, std::move(UArgs), + std::move(TranslatedArgs), ContainsError); if (!HandleImmediateArgs(*C)) return C; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits