jansvoboda11 marked 3 inline comments as done. jansvoboda11 added inline comments.
================ 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); ---------------- vsapsai wrote: > 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. I agree the order is sometimes a bit weird. It seems like the convention is to pass "bigger" objects (like `Preprocessor`, `HeaderSearch`, `CompilerInstance` etc.) first. I'll move the new out param to the back, but probably won't mess with the existing parameters. ================ 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); ---------------- vsapsai wrote: > 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. It's also called twice in `Preprocessor::EnterMainSourceFile` where specifying `true` would seem redundant. I think I'll keep it as is for now. 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
