vsapsai added a comment. Have just a few little tweaks, otherwise looks good. And we've discussed that it's possible we don't really need the first time lexing check anymore. But that's a separate issue and I support your decision to preserve the existing behavior and not to increase the scope of work.
================ Comment at: clang/include/clang/Lex/HeaderSearch.h:445-447 + bool ShouldEnterIncludeFile(bool &IsFirstIncludeOfFile, Preprocessor &PP, + const FileEntry *File, bool isImport, + bool ModulesEnabled, Module *M); ---------------- As we've discussed earlier, the order of parameters is slightly weird. Usually, out-parameters are at the end of the list, though I'm not sure there is a rule about it. Personally, I would move `File` to the front because the method is ShouldEnter**IncludeFile** and keep the order as `File, isImport, PP, ModulesEnabled, M, IsFirstIncludeOfFile` but that's more my personal opinion and I'll leave the final decision to you. ================ Comment at: clang/include/clang/Lex/Preprocessor.h:1370-1371 /// Emits a diagnostic, doesn't enter the file, and returns true on error. bool EnterSourceFile(FileID FID, const DirectoryLookup *Dir, - SourceLocation Loc); + SourceLocation Loc, bool IsFirstIncludeOfFile = true); ---------------- Not sure we need a default value for `IsFirstIncludeOfFile`. Grepping shows the only other place the method is called is `IncrementalParser::Parse`, so we can provide an explicit value there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114093/new/ https://reviews.llvm.org/D114093 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
