Skip to content

Commit aa1bb82

Browse files
Elvand-Lievgvassilev
authored andcommitted
Use unique_ptr for ReverseModeVisitor::m_ExternalSource
Replace the raw owning pointer m_ExternalSource with std::unique_ptr<MultiplexExternalRMVSource>. This makes ownership explicit and exception-safe, removing the manual delete path in the destructor. The destructor is now defaulted. All existing usage sites use either bool-test (if (m_ExternalSource)) or arrow dereference (m_ExternalSource->Method()), both of which work identically with unique_ptr. Fixes: #1839
1 parent 319a87a commit aa1bb82

2 files changed

Lines changed: 6 additions & 15 deletions

File tree

include/clad/Differentiator/ReverseModeVisitor.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,10 @@ namespace clad {
6464
// a separate namespace, as well as add getters/setters function of
6565
// several private/protected members of the visitor classes.
6666
friend class ErrorEstimationHandler;
67-
// FIXME: Should we make this an object instead of a pointer?
68-
// Downside of making it an object: We will need to include
69-
// 'MultiplexExternalRMVSource.h' file
70-
MultiplexExternalRMVSource* m_ExternalSource = nullptr;
67+
// External sources are owned by the visitor and tied to the current
68+
// derivative generation run. Keep them in a unique_ptr to avoid manual
69+
// delete paths and stale ownership.
70+
std::unique_ptr<MultiplexExternalRMVSource> m_ExternalSource;
7171
llvm::SmallVector<const clang::ParmVarDecl*, 16> m_NonIndepParams;
7272
/// In addition to a sequence of forward-accumulated Stmts (m_Blocks), in
7373
/// the reverse mode we also accumulate Stmts for the reverse pass which

lib/Differentiator/ReverseModeVisitor.cpp

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -252,16 +252,7 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
252252
const DiffRequest& request)
253253
: VisitorBase(builder, request) {}
254254

255-
ReverseModeVisitor::~ReverseModeVisitor() {
256-
if (m_ExternalSource) {
257-
// Inform external sources that `ReverseModeVisitor` object no longer
258-
// exists.
259-
// FIXME: Make this so the lifetime scope of the source matches.
260-
// m_ExternalSource->ForgetRMV();
261-
// Free the external sources multiplexer since we own this resource.
262-
delete m_ExternalSource;
263-
}
264-
}
255+
ReverseModeVisitor::~ReverseModeVisitor() = default;
265256

266257
DerivativeAndOverload ReverseModeVisitor::Derive() {
267258
assert(m_DiffReq.Function && "Must not be null.");
@@ -4494,7 +4485,7 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
44944485

44954486
void ReverseModeVisitor::AddExternalSource(ExternalRMVSource& source) {
44964487
if (!m_ExternalSource)
4497-
m_ExternalSource = new MultiplexExternalRMVSource();
4488+
m_ExternalSource = std::make_unique<MultiplexExternalRMVSource>();
44984489
source.InitialiseRMV(*this);
44994490
m_ExternalSource->AddSource(source);
45004491
}

0 commit comments

Comments
 (0)