[PATCH] D114093: [clang][lex] Refactor check for the first file include

[Volodymyr Sapsai via Phabricator via cfe-commits]

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