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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits