- 
                Notifications
    You must be signed in to change notification settings 
- Fork 15k
          [support] Don't require VFS in SourceMgr for loading includes
          #163862
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
  
    [support] Don't require VFS in SourceMgr for loading includes
  
  #163862
              Conversation
This commit more gracefully handles situations where `SourceMgr` isn't initialized with a VFS and tries to resolve an include. That's what happens with the test case `clang -flto=thin -c test.c -o /dev/null` where test.c contains `asm(" .incbin \"foo.i\" \n");`. Propagating the actual VFS all the way is very difficult.
This is a follow-up to llvm#162903.
    | @llvm/pr-subscribers-llvm-support Author: Jan Svoboda (jansvoboda11) ChangesThis commit more gracefully handles situations where  This is a follow-up to #162903. Full diff: https://github.com/llvm/llvm-project/pull/163862.diff 1 Files Affected: 
 diff --git a/llvm/lib/Support/SourceMgr.cpp b/llvm/lib/Support/SourceMgr.cpp
index f2bbaab23ed7b..f2ceaa0c9a669 100644
--- a/llvm/lib/Support/SourceMgr.cpp
+++ b/llvm/lib/Support/SourceMgr.cpp
@@ -69,11 +69,11 @@ unsigned SourceMgr::AddIncludeFile(const std::string &Filename,
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 SourceMgr::OpenIncludeFile(const std::string &Filename,
                            std::string &IncludedFile) {
-  if (!FS)
-    reportFatalInternalError("Opening include file from SourceMgr without VFS");
+  auto getFile = [this](StringRef Path) {
+    return FS ? FS->getBufferForFile(Path) : MemoryBuffer::getFile(Path);
+  };
 
-  ErrorOr<std::unique_ptr<MemoryBuffer>> NewBufOrErr =
-      FS->getBufferForFile(Filename);
+  ErrorOr<std::unique_ptr<MemoryBuffer>> NewBufOrErr = getFile(Filename);
 
   SmallString<64> Buffer(Filename);
   // If the file didn't exist directly, see if it's in an include path.
@@ -81,7 +81,7 @@ SourceMgr::OpenIncludeFile(const std::string &Filename,
        ++i) {
     Buffer = IncludeDirectories[i];
     sys::path::append(Buffer, Filename);
-    NewBufOrErr = FS->getBufferForFile(Buffer);
+    NewBufOrErr = getFile(Buffer);
   }
 
   if (NewBufOrErr)
 | 
| Looking forward to getting this merged :). It's currently breaking one of our tools. | 
| Thanks, this obviously resolves the failure since it deletes it and the include still works. | 
With llvm#163862, this is not really necessary and causes downstream issues.
With llvm#163862, this is not really necessary and causes downstream issues.
With llvm#163862, this is not really necessary and causes downstream issues.
This commit more gracefully handles situations where
SourceMgrisn't initialized with a VFS and tries to resolve an include. That's what happens with the test caseclang -flto=thin -c test.c -o /dev/nullwhere test.c containsasm(" .incbin \"foo.i\" \n");. Propagating the actual VFS all the way is very difficult.This is a follow-up to #162903.