jansvoboda11 created this revision.
jansvoboda11 added reviewers: ahoppen, Bigcheese, dexonsmith.
Herald added subscribers: usaxena95, shchenz, kadircet, arphaman, kbarton, 
nemanjai.
jansvoboda11 requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119708

Files:
  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

Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1845,28 +1845,6 @@
   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;
 
Index: clang/include/clang/Lex/PPCallbacks.h
===================================================================
--- clang/include/clang/Lex/PPCallbacks.h
+++ clang/include/clang/Lex/PPCallbacks.h
@@ -61,23 +61,6 @@
                            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 @@
     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,
Index: clang-tools-extra/pp-trace/PPCallbacksTracker.h
===================================================================
--- clang-tools-extra/pp-trace/PPCallbacksTracker.h
+++ clang-tools-extra/pp-trace/PPCallbacksTracker.h
@@ -91,8 +91,6 @@
                    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,
Index: clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
===================================================================
--- clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
+++ clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
@@ -128,16 +128,6 @@
   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.
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -226,10 +226,6 @@
           /*Imported=*/nullptr, Inc.FileKind);
       if (File)
         Delegate->FileSkipped(*File, SynthesizedFilenameTok, Inc.FileKind);
-      else {
-        llvm::SmallString<1> UnusedRecovery;
-        Delegate->FileNotFound(WrittenFilename, UnusedRecovery);
-      }
     }
   }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to