[PATCH] unique_ptrify ownership of ModuleFiles in the ModuleManager
Richard Smith
richard at metafoo.co.uk
Fri Oct 24 13:18:02 PDT 2014
I think this is correct. I'm on the fence as to whether it's an improvement, due to the sort-of dual ownership of these pointers.
================
Comment at: include/clang/Serialization/ModuleManager.h:34-37
@@ -33,6 +33,6 @@
/// user, the last one is the one that doesn't depend on anything further.
SmallVector<ModuleFile *, 2> Chain;
/// \brief All loaded modules, indexed by name.
- llvm::DenseMap<const FileEntry *, ModuleFile *> Modules;
+ llvm::DenseMap<const FileEntry *, std::unique_ptr<ModuleFile>> Modules;
----------------
Storing pointers to `ModuleFile`s that point into values owned by a `unique_ptr` worries me a little. Seems like there's a risk here of a dangling pointer getting left in `Chain`; I think it'd be more obvious this happened if the `delete` were explicit.
http://reviews.llvm.org/D5980
More information about the cfe-commits
mailing list