[clang-tools-extra] 7a124f4 - [clang][lex] Remove `PPCallbacks::FileNotFound()`

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2022-02-15T09:48:25+01:00
New Revision: 7a124f4859d5c4093467abc2e734f8ab15e78cc6

URL: https://github.com/llvm/llvm-project/commit/7a124f4859d5c4093467abc2e734f8ab15e78cc6
DIFF: https://github.com/llvm/llvm-project/commit/7a124f4859d5c4093467abc2e734f8ab15e78cc6.diff

LOG: [clang][lex] Remove `PPCallbacks::FileNotFound()`

The purpose of the `FileNotFound` preprocessor callback was to add the ability to recover from failed header lookups. This was to support downstream project.

However, injecting additional search path while performing header search can invalidate currently used iterators/references to `DirectoryLookup` in `Preprocessor` and `HeaderSearch`.

The downstream project ended up maintaining a separate patch to further tweak the functionality. Since we don't have any upstream users nor open source downstream users, I'd like to remove this callback for good to prevent future misuse. I doubt there are any actual downstream users, since the functionality is definitely broken at the moment.

Reviewed By: ahoppen

Differential Revision: https://reviews.llvm.org/D119708

Added: 
    

Modified: 
    clang-tools-extra/clangd/ParsedAST.cpp
    clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
    clang-tools-extra/pp-trace/PPCallbacksTracker.h
    clang/include/clang/Lex/PPCallbacks.h
    clang/lib/Lex/PPDirectives.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 86c4b2151243..199cad8dde44 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -226,10 +226,6 @@ class ReplayPreamble : private PPCallbacks {
           /*Imported=*/nullptr, Inc.FileKind);
       if (File)
         Delegate->FileSkipped(*File, SynthesizedFilenameTok, Inc.FileKind);
-      else {
-        llvm::SmallString<1> UnusedRecovery;
-        Delegate->FileNotFound(WrittenFilename, UnusedRecovery);
-      }
     }
   }
 

diff  --git a/clang-tools-extra/pp-trace/PPCallbacksTracker.cpp b/clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
index 63faae651998..785fa9b679bb 100644
--- a/clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
+++ b/clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
@@ -128,16 +128,6 @@ void PPCallbacksTracker::FileSkipped(const FileEntryRef &SkippedFile,
   appendArgument("FileType", FileType, CharacteristicKindStrings);
 }
 
-// Callback invoked whenever an inclusion directive results in a
-// file-not-found error.
-bool
-PPCallbacksTracker::FileNotFound(llvm::StringRef FileName,
-                                 llvm::SmallVectorImpl<char> &RecoveryPath) {
-  beginCallback("FileNotFound");
-  appendFilePathArgument("FileName", FileName);
-  return false;
-}
-
 // Callback invoked whenever an inclusion directive of
 // any kind (#include, #import, etc.) has been processed, regardless
 // of whether the inclusion will actually result in an inclusion.

diff  --git a/clang-tools-extra/pp-trace/PPCallbacksTracker.h b/clang-tools-extra/pp-trace/PPCallbacksTracker.h
index db5d51b00364..7be31031fe71 100644
--- a/clang-tools-extra/pp-trace/PPCallbacksTracker.h
+++ b/clang-tools-extra/pp-trace/PPCallbacksTracker.h
@@ -91,8 +91,6 @@ class PPCallbacksTracker : public PPCallbacks {
                    FileID PrevFID = FileID()) override;
   void FileSkipped(const FileEntryRef &SkippedFile, const Token &FilenameTok,
                    SrcMgr::CharacteristicKind FileType) override;
-  bool FileNotFound(llvm::StringRef FileName,
-                    llvm::SmallVectorImpl<char> &RecoveryPath) override;
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           llvm::StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,

diff  --git a/clang/include/clang/Lex/PPCallbacks.h b/clang/include/clang/Lex/PPCallbacks.h
index 76a74f20cc3b..298947836a39 100644
--- a/clang/include/clang/Lex/PPCallbacks.h
+++ b/clang/include/clang/Lex/PPCallbacks.h
@@ -61,23 +61,6 @@ class PPCallbacks {
                            const Token &FilenameTok,
                            SrcMgr::CharacteristicKind FileType) {}
 
-  /// Callback invoked whenever an inclusion directive results in a
-  /// file-not-found error.
-  ///
-  /// \param FileName The name of the file being included, as written in the
-  /// source code.
-  ///
-  /// \param RecoveryPath If this client indicates that it can recover from
-  /// this missing file, the client should set this as an additional header
-  /// search patch.
-  ///
-  /// \returns true to indicate that the preprocessor should attempt to recover
-  /// by adding \p RecoveryPath as a header search path.
-  virtual bool FileNotFound(StringRef FileName,
-                            SmallVectorImpl<char> &RecoveryPath) {
-    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.
@@ -443,12 +426,6 @@ class PPChainedCallbacks : public PPCallbacks {
     Second->FileSkipped(SkippedFile, FilenameTok, FileType);
   }
 
-  bool FileNotFound(StringRef FileName,
-                    SmallVectorImpl<char> &RecoveryPath) override {
-    return First->FileNotFound(FileName, RecoveryPath) ||
-           Second->FileNotFound(FileName, RecoveryPath);
-  }
-
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index ae5e4fc75c15..a7201490e4b1 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -1845,28 +1845,6 @@ Optional<FileEntryRef> Preprocessor::LookupHeaderIncludeOrImport(
   if (File)
     return File;
 
-  if (Callbacks) {
-    // Give the clients a chance to recover.
-    SmallString<128> RecoveryPath;
-    if (Callbacks->FileNotFound(Filename, RecoveryPath)) {
-      if (auto DE = FileMgr.getOptionalDirectoryRef(RecoveryPath)) {
-        // Add the recovery path to the list of search paths.
-        DirectoryLookup DL(*DE, SrcMgr::C_User, false);
-        HeaderInfo.AddSearchPath(DL, isAngled);
-
-        // Try the lookup again, skipping the cache.
-        Optional<FileEntryRef> File = LookupFile(
-            FilenameLoc,
-            LookupFilename, isAngled,
-            LookupFrom, LookupFromFile, CurDir, nullptr, nullptr,
-            &SuggestedModule, &IsMapped, /*IsFrameworkFound=*/nullptr,
-            /*SkipCache*/ true);
-        if (File)
-          return File;
-      }
-    }
-  }
-
   if (SuppressIncludeNotFoundError)
     return None;