[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:45:00 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 1/2] [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;
>From de3a9a2fa6ed4f20c5ca3cac346d924485e683c3 Mon Sep 17 00:00:00 2001
From: Victor Campos <victor.campos at arm.com>
Date: Fri, 15 Nov 2024 16:43:57 +0000
Subject: [PATCH 2/2] Minor changes to make it better
---
clang/lib/Driver/Compilation.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp
index b8025d6a46f2e6..c13790196c1bdd 100644
--- a/clang/lib/Driver/Compilation.cpp
+++ b/clang/lib/Driver/Compilation.cpp
@@ -77,7 +77,7 @@ Compilation::getArgsForToolChain(const ToolChain *TC, StringRef BoundArch,
NewDAL.reset(TC->TranslateXarchArgs(*OpenMPArgs, BoundArch,
DeviceOffloadKind, &AllocatedArgs));
if (!NewDAL)
- NewDAL = OpenMPArgs;
+ NewDAL = std::move(OpenMPArgs);
}
if (!NewDAL) {
@@ -88,7 +88,7 @@ Compilation::getArgsForToolChain(const ToolChain *TC, StringRef BoundArch,
} else {
Entry.reset(TC->TranslateArgs(*NewDAL, BoundArch, DeviceOffloadKind));
if (!Entry)
- Entry = std::shared_ptr<DerivedArgList>(NewDAL);
+ Entry = std::move(NewDAL);
}
// Add allocated arguments to the final DAL.
More information about the cfe-commits
mailing list