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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
