vsapsai added inline comments.
================ Comment at: clang/include/clang/Lex/Preprocessor.h:1251-1260 + const IncludedFilesSet *getNullSubmoduleIncludes() const { + auto It = IncludedFilesPerSubmodule.find(nullptr); + return It == IncludedFilesPerSubmodule.end() ? nullptr : &It->second; } - /// Get the set of included files. - IncludedFilesSet &getIncludedFiles() { return IncludedFiles; } - const IncludedFilesSet &getIncludedFiles() const { return IncludedFiles; } + /// Get the set of files included in the given (sub)module. + const IncludedFilesSet *getLocalSubmoduleIncludes(Module *M) const { ---------------- Was curious why `getNullSubmoduleIncludes` isn't `getLocalSubmoduleIncludes(nullptr)`? If we want to have separate methods and implementations, it might be useful to assert `M` isn't null in `getLocalSubmoduleIncludes`. ================ Comment at: clang/lib/Serialization/ASTReader.cpp:8631 +const Preprocessor::IncludedFilesSet *ASTReader::getIncludedFiles(Module *M) { + ModuleFile *F = getModuleManager().lookup(M->getASTFile()); ---------------- Can you please check again the returned pointer doesn't end up as a dangling pointer? I don't think we store the pointer anywhere, which is good. My bigger concern is if we can invalidate `SubmoduleIncludedFiles` iterator while working with the returned pointer. I haven't found any indication of that but would like somebody else to check that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112915/new/ https://reviews.llvm.org/D112915 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits