[PATCH] D65956: clang: Diag running out of file handles while looking for files

[Axel Naumann via Phabricator via cfe-commits]

karies added a comment.
Herald added a project: All.

Similar to the concern raised at https://github.com/llvm/llvm-project/commit/50fcf7285eeb001d751eadac5d001a0e216fd701 we have received user reports about this patch: with `-Ino-access-permissions -Iall-good`, clang will throw an error (EACCES) even though header search goes on and will find the header in `all-good`. That seems a misleading an unnecessary error, especially as the header *is* found later, yet compilation "fails" because of this diagnostic.

I would propose to collect these errors, and where originally (before this patch), cling would complain "file not found" we could diagnose "while searching for this header, the following errors have been seen" and reporting the uniq-ed set of collected diags - but *only* in the case where the file cannot be found. This would prevent spurious diags during iteration through the SearchDirs when HeaderSearch actually finds the file. And I am fully aware that it's pointless to propose something without providing a differential :-/

FYI here's what we do right now to handle the EACCES case:

  patch
  --- a/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
  +++ b/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
  @@ -367,7 +367,9 @@ Optional<FileEntryRef> HeaderSearch::getFileAndSuggestModule(
       std::error_code EC = llvm::errorToErrorCode(File.takeError());
       if (EC != llvm::errc::no_such_file_or_directory &&
           EC != llvm::errc::invalid_argument &&
  -        EC != llvm::errc::is_a_directory && EC != llvm::errc::not_a_directory) {
  +        EC != llvm::errc::is_a_directory &&
  +        EC != llvm::errc::not_a_directory &&
  +        EC != llvm::errc::permission_denied) {
         Diags.Report(IncludeLoc, diag::err_cannot_open_file)
             << FileName << EC.message();
       }


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65956/new/

https://reviews.llvm.org/D65956