================ 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; ---------------- rsmith wrote: > 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. Not sure how much more obvious it'd be - as it stands the likely problem is a memory leak (such as 220569 ) rather than an inconsistency between these two data structures.
At least if something's dangling it'll likely fail pretty quick, rather than silently leak... but yeah, tradeoffs. This certainly isn't the only case where we have owning and non-owning data structures at the same scope (including member scope), so I don't think it's all that surprising, but I could be wrong. Certainly we could comment the members more clearly to indicate that the poniters of one point to the same objects as the unique_ptrs of the other and that these need to be kept in step (this latter property is probably worth describing in more detail regardless) http://reviews.llvm.org/D5980 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
