[clang] [clang][Driver] Use shared_ptr in the Compilation class (PR #116406)

Victor Campos via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 15 08:39:55 PST 2024


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.campos at 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;



More information about the cfe-commits mailing list