https://github.com/DavidTruby commented:
I think you still don't need any of the shared_ptr usage here, there's no
shared ownership semantics. If you change all these to unique_ptr then I think
the patch would look good to me.
https://github.com/llvm/llvm-project/pull/116406
__
@@ -61,11 +61,11 @@ class Compilation {
OrderedOffloadingToolchains;
/// The original (untranslated) input argument list.
- llvm::opt::InputArgList *Args;
+ std::unique_ptr Args;
/// The driver translated arguments. Note that toolchains may perform their
///
@@ -63,41 +56,39 @@ Compilation::getArgsForToolChain(const ToolChain *TC,
StringRef BoundArch,
if (!TC)
TC = &DefaultToolChain;
- DerivedArgList *&Entry = TCArgs[{TC, BoundArch, DeviceOffloadKind}];
+ std::shared_ptr &Entry =
+ TCArgs[{TC, BoundArch, DeviceOffloa
@@ -63,41 +56,39 @@ Compilation::getArgsForToolChain(const ToolChain *TC,
StringRef BoundArch,
if (!TC)
TC = &DefaultToolChain;
- DerivedArgList *&Entry = TCArgs[{TC, BoundArch, DeviceOffloadKind}];
+ std::shared_ptr &Entry =
DavidTruby wrote:
You ca
@@ -100,7 +100,7 @@ class Compilation {
return false;
}
};
- std::map TCArgs;
+ std::map> TCArgs;
DavidTruby wrote:
This can be a unique_ptr, as TCArgs owns the underlying DerivedArgList.
https://github.com/llvm/llvm-project/pull/116406
@@ -63,41 +56,39 @@ Compilation::getArgsForToolChain(const ToolChain *TC,
StringRef BoundArch,
if (!TC)
TC = &DefaultToolChain;
- DerivedArgList *&Entry = TCArgs[{TC, BoundArch, DeviceOffloadKind}];
+ std::shared_ptr &Entry =
+ TCArgs[{TC, BoundArch, DeviceOffloa
https://github.com/DavidTruby edited
https://github.com/llvm/llvm-project/pull/116406
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/MaskRay requested changes to this pull request.
Using unique_ptr for `Args` is good, but I am not sure about shared_ptr for
TranslatedArgs. There is no shared_ptr usage in clangDriver and we want to keep
avoiding it.
https://github.com/llvm/llvm-project/pull/116406
_
@@ -1544,8 +1544,8 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) {
}
// The compilation takes ownership of Args.
- Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs,
- ContainsError);
+ Compilation
https://github.com/DavidTruby edited
https://github.com/llvm/llvm-project/pull/116406
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DavidTruby requested changes to this pull request.
Thanks for the patch!
I don't think there's any actual shared ownership happening here so I think you
could use `unique_ptr` instead, and that has lower overhead so would be
preferable I think.
I've left a comment as to why
llvmbot wrote:
@llvm/pr-subscribers-clang-driver
Author: Victor Campos (vhscampos)
Changes
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 sim
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
Date: Fri, 15 Nov 2024 16:15:04 +
Subject: [PATCH] [clang][Driver] Use shared_ptr in the Compilation class
This
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
Date: Fri, 15 Nov 2024 16:15:04 +
Subject: [PATCH 1/2] [clang][Driver] Use shared_ptr in the Compilation class
T
https://github.com/vhscampos created
https://github.com/llvm/llvm-project/pull/116406
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.
>Fro
15 matches
Mail list logo
