ahoppen added a comment. LGTM overall. I’ve got a few questions/remarks inline.
================ Comment at: clang/lib/Lex/HeaderSearch.cpp:94 + // Module map parsing initiated by header search. + if (HS.CurrentSearchPathIdx != ~0U) + HS.ModuleToSearchDirIdx[M] = HS.CurrentSearchPathIdx; ---------------- When would the `moduleMapModuleCreated` be called while `CurrentSearchPathIdx == ~0U`? Could this be an `assert` instead? ================ Comment at: clang/lib/Lex/HeaderSearch.cpp:264 + if (Module || !AllowSearch || !HSOpts->ImplicitModuleMaps) { + noteModuleLookupUsage(Module, ImportLoc); return Module; ---------------- Just a thought: Could we move `noteModuleLookupUsage` into `findModule`? IIUC that would guarantee that we’re catching all cases and we wouldn’t need to call `noteModuleLookupUsage ` from both overloads of `lookupModule`. ================ Comment at: clang/lib/Lex/ModuleMap.cpp:851 new Module("<private>", Loc, Parent, /*IsFramework*/ false, /*IsExplicit*/ true, NumCreatedModules++); Result->Kind = Module::PrivateModuleFragment; ---------------- Maybe a stupid question but why don’t we need to call the callback e.g. here (or on the other `new Module`) calls in this file? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113676/new/ https://reviews.llvm.org/D113676 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits