[clang][Lex] Add back PPCallbacks::FileNotFound

[Hahnfeld]

Summary

This callback was removed with commit rG7a124f4859, but we use it downstream in ROOT/Cling to implement handling of a special include syntax. Add back a "safe" version of the callback that only takes the file name and return a bool to silently skip the file.

[Hahnfeld]

I just checked and this "simplified" version of the callback would give us enough information to implement all we need in ROOT (plus some, the bool return value is nicer than temporarily playing with SuppressIncludeNotFoundError). For an upstream test, I could implement a unit test in clang/unittests/Lex/PPCallbacksTest.cpp or a "toy" tooling where #include <file.h?> is ignored if file.h doesn't exist? What do you think (assuming the general idea of adding this callback is fine)?

[jansvoboda11]

In D142196#4068428, @Hahnfeld wrote:

I just checked and this "simplified" version of the callback would give us enough information to implement all we need in ROOT (plus some, the bool return value is nicer than temporarily playing with SuppressIncludeNotFoundError). For an upstream test, I could implement a unit test in clang/unittests/Lex/PPCallbacksTest.cpp or a "toy" tooling where #include <file.h?> is ignored if file.h doesn't exist? What do you think (assuming the general idea of adding this callback is fine)?

Unit test in clang/unittests/Lex/PPCallbacksTest.cpp sounds good, go for it!

[Hahnfeld]

Add unit test.

[jansvoboda11]

LGTM with improved wording.

clang/include/clang/Lex/PPCallbacks.h

87

The wording is a bit misleading due to:

if (SuppressIncludeNotFoundError)
  return std::nullopt;

after invoking this callback. Reading the comment, I assume the diagnostic has already been issued. Can we tweak this to clarify things?

[jansvoboda11]

Also I think we should mention this API in our docs, right where the removed original used to be.

[Hahnfeld]

Update comment.

[Hahnfeld]

In D142196#4074249, @jansvoboda11 wrote:

Also I think we should mention this API in our docs, right where the removed original used to be.

Which API docs are you referring to? I'm only aware of the pp-trace docs (updated in https://reviews.llvm.org/D125258 for the removal), but I didn't add FileNotFound to pp-trace because it's more of a hook than a "traceable event" - should I add it nevertheless?

clang/include/clang/Lex/PPCallbacks.h

87

Please let me know if this sounds better.

[Hahnfeld]

I checked a second time and couldn't find other API docs. I've now pushed this change for now (also to get it included in release/16.x), but please let me know for any follow-ups. Thanks!

[jansvoboda11]

Yes, I meant the pp-trace docs. Why would "we didn't find the included file" not be a traceable event?

[Hahnfeld]

In D142196#4077708, @jansvoboda11 wrote:

Yes, I meant the pp-trace docs. Why would "we didn't find the included file" not be a traceable event?

My thinking is the following: If the preprocessor cannot find the included file and the Hook doesn't return true, it will run into a fatal error and stop parsing the source file. I don't know if that's actually a relevant case, but I have no experience with pp-trace, so maybe?

clang/include/clang/Lex/PPCallbacks.h

public:
   ///
   /// \param FilenameTok The file name token in \#include "FileName" directive
   /// or macro expanded file name token from \#include MACRO(PARAMS) directive.
   /// Note that FilenameTok contains corresponding quotes/angles symbols.
   virtual void FileSkipped(const FileEntryRef &SkippedFile,
                            const Token &FilenameTok,
                            SrcMgr::CharacteristicKind FileType) {}
 
+  /// Callback invoked whenever the preprocessor cannot find a file for an
+  /// inclusion directive.
+  ///
+  /// \param FileName The name of the file being included, as written in the
+  /// source code.
+  ///
+  /// \returns true to indicate that the preprocessor should skip this file
+  /// and not issue any diagnostic.
+  virtual bool FileNotFound(StringRef FileName) { return false; }
+
   /// Callback invoked whenever an inclusion directive of
   /// any kind (\c \#include, \c \#import, etc.) has been processed, regardless
   /// of whether the inclusion will actually result in an inclusion.
   ///
   /// \param HashLoc The location of the '#' that starts the inclusion
   /// directive.
   ///
   /// \param IncludeTok The token that indicates the kind of inclusion
public:
   }
 
   void FileSkipped(const FileEntryRef &SkippedFile, const Token &FilenameTok,
                    SrcMgr::CharacteristicKind FileType) override {
     First->FileSkipped(SkippedFile, FilenameTok, FileType);
     Second->FileSkipped(SkippedFile, FilenameTok, FileType);
   }
 
+  bool FileNotFound(StringRef FileName) override {
+    bool Skip = First->FileNotFound(FileName);
+    // Make sure to invoke the second callback, no matter if the first already
+    // returned true to skip the file.
+    Skip |= Second->FileNotFound(FileName);
+    return Skip;
+  }
+
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef File, StringRef SearchPath,
                           StringRef RelativePath, const Module *Imported,
                           SrcMgr::CharacteristicKind FileType) override {
     First->InclusionDirective(HashLoc, IncludeTok, FileName, IsAngled,
                               FilenameRange, File, SearchPath, RelativePath,

clang/lib/Lex/PPDirectives.cpp

OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport(
     ModuleMap::KnownHeader &SuggestedModule, bool isAngled) {
   OptionalFileEntryRef File = LookupFile(
       FilenameLoc, LookupFilename, isAngled, LookupFrom, LookupFromFile, CurDir,
       Callbacks ? &SearchPath : nullptr, Callbacks ? &RelativePath : nullptr,
       &SuggestedModule, &IsMapped, &IsFrameworkFound);
   if (File)
     return File;
 
+  // Give the clients a chance to silently skip this include.
+  if (Callbacks && Callbacks->FileNotFound(Filename))
+    return std::nullopt;
+
   if (SuppressIncludeNotFoundError)
     return std::nullopt;
 
   // If the file could not be located and it was included via angle
   // brackets, we can attempt a lookup as though it were a quoted path to
   // provide the user with a possible fixit.
   if (isAngled) {
     OptionalFileEntryRef File = LookupFile(

clang/unittests/Lex/PPCallbacksTest.cpp

const char* Source =
     "#include MACRO_TRIGRAPH\n";
 
   CharSourceRange Range =
     InclusionDirectiveFilenameRange(Source, "/tri~graph.h", false);
 
   ASSERT_EQ("\"tri\?\?-graph.h\"", GetSourceString(Range));
 }
 
+TEST_F(PPCallbacksTest, FileNotFoundSkipped) {
+  const char *SourceText = "#include \"skipped.h\"\n";
+
+  std::unique_ptr<llvm::MemoryBuffer> SourceBuf =
+      llvm::MemoryBuffer::getMemBuffer(SourceText);
+  SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(SourceBuf)));
+
+  HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
+                          Diags, LangOpts, Target.get());
+  TrivialModuleLoader ModLoader;
+
+  DiagnosticConsumer *DiagConsumer = new DiagnosticConsumer;
+  DiagnosticsEngine FileNotFoundDiags(DiagID, DiagOpts.get(), DiagConsumer);
+  Preprocessor PP(std::make_shared<PreprocessorOptions>(), FileNotFoundDiags,
+                  LangOpts, SourceMgr, HeaderInfo, ModLoader,
+                  /*IILookup=*/nullptr,
+                  /*OwnsHeaderSearch=*/false);
+  PP.Initialize(*Target);
+
+  class FileNotFoundCallbacks : public PPCallbacks {
+  public:
+    unsigned int NumCalls = 0;
+    bool FileNotFound(StringRef FileName) override {
+      NumCalls++;
+      return FileName == "skipped.h";
+    }
+  };
+
+  auto *Callbacks = new FileNotFoundCallbacks;
+  PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));
+
+  // Lex source text.
+  PP.EnterMainSourceFile();
+  while (true) {
+    Token Tok;
+    PP.Lex(Tok);
+    if (Tok.is(tok::eof))
+      break;
+  }
+
+  ASSERT_EQ(1u, Callbacks->NumCalls);
+  ASSERT_EQ(0u, DiagConsumer->getNumErrors());
+}
+
 TEST_F(PPCallbacksTest, OpenCLExtensionPragmaEnabled) {
   const char* Source =
     "#pragma OPENCL EXTENSION cl_khr_fp64 : enable\n";
 
   PragmaOpenCLExtensionCallbacks::CallbackParameters Parameters =
     PragmaOpenCLExtensionCall(Source);
 
   ASSERT_EQ("cl_khr_fp64", Parameters.Name);