On Tue, Sep 5, 2017 at 2:47 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> Author: rsmith > Date: Tue Sep 5 14:46:22 2017 > New Revision: 312580 > > URL: http://llvm.org/viewvc/llvm-project?rev=312580&view=rev > Log: > Fix memory leak after r312467. The ModuleMap is the owner of the global > module object until it's reparented under a real module. > > Modified: > cfe/trunk/include/clang/Lex/ModuleMap.h > cfe/trunk/lib/Lex/ModuleMap.cpp > > Modified: cfe/trunk/include/clang/Lex/ModuleMap.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=312580&r1=312579&r2=312580&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Lex/ModuleMap.h (original) > +++ cfe/trunk/include/clang/Lex/ModuleMap.h Tue Sep 5 14:46:22 2017 > @@ -82,22 +82,26 @@ class ModuleMap { > > /// \brief The directory used for Clang-supplied, builtin include > headers, > /// such as "stdint.h". > - const DirectoryEntry *BuiltinIncludeDir; > + const DirectoryEntry *BuiltinIncludeDir = nullptr; > > /// \brief Language options used to parse the module map itself. > /// > /// These are always simple C language options. > LangOptions MMapLangOpts; > > - // The module that the main source file is associated with (the module > - // named LangOpts::CurrentModule, if we've loaded it). > - Module *SourceModule; > + /// The module that the main source file is associated with (the module > + /// named LangOpts::CurrentModule, if we've loaded it). > + Module *SourceModule = nullptr; > + > + /// The global module for the current TU, if we still own it. > (Ownership is > + /// transferred if/when we create an enclosing module. > + std::unique_ptr<Module> PendingGlobalModule; > > /// \brief The top-level modules that are known. > llvm::StringMap<Module *> Modules; > > /// \brief The number of modules we have created in total. > - unsigned NumCreatedModules; > + unsigned NumCreatedModules = 0; > > public: > /// \brief Flags describing the role of a module header. > > Modified: cfe/trunk/lib/Lex/ModuleMap.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=312580&r1=312579&r2=312580&view=diff > > ============================================================================== > --- cfe/trunk/lib/Lex/ModuleMap.cpp (original) > +++ cfe/trunk/lib/Lex/ModuleMap.cpp Tue Sep 5 14:46:22 2017 > @@ -256,8 +256,7 @@ ModuleMap::ModuleMap(SourceManager &Sour > const LangOptions &LangOpts, const TargetInfo > *Target, > HeaderSearch &HeaderInfo) > : SourceMgr(SourceMgr), Diags(Diags), LangOpts(LangOpts), > Target(Target), > - HeaderInfo(HeaderInfo), BuiltinIncludeDir(nullptr), > - SourceModule(nullptr), NumCreatedModules(0) { > + HeaderInfo(HeaderInfo) { > MMapLangOpts.LineComment = true; > } > > @@ -747,10 +746,12 @@ std::pair<Module *, bool> ModuleMap::fin > } > > Module *ModuleMap::createGlobalModuleForInterfaceUnit(SourceLocation Loc) > { > - auto *Result = new Module("<global>", Loc, nullptr, /*IsFramework*/ > false, > - /*IsExplicit*/ true, NumCreatedModules++); > - Result->Kind = Module::GlobalModuleFragment; > - return Result; > + assert(!PendingGlobalModule && "created multiple global modules"); > + PendingGlobalModule.reset( > + new Module("<global>", Loc, nullptr, /*IsFramework*/ false, > + /*IsExplicit*/ true, NumCreatedModules++)); > + PendingGlobalModule->Kind = Module::GlobalModuleFragment; > + return PendingGlobalModule.get(); > } > > Module *ModuleMap::createModuleForInterfaceUnit(SourceLocation Loc, > @@ -766,7 +767,10 @@ Module *ModuleMap::createModuleForInterf > Modules[Name] = SourceModule = Result; > > // Reparent the current global module fragment as a submodule of this > module. > + assert(GlobalModule == PendingGlobalModule.get() && > + "unexpected global module"); > GlobalModule->setParent(Result); > + PendingGlobalModule.release(); // now owned by parent > Would it be reasonable to reverse the 'setParent' API (into an addSubmodule call on the parent - that takes the child/submodule by std::unique_ptr value) so the hand-off/ownership is more clear/less error prone? > > // Mark the main source file as being within the newly-created module > so that > // declarations and macros are properly visibility-restricted to it. > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
