[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