Skip to content

Conversation

@tcwzxx
Copy link
Member

@tcwzxx tcwzxx commented Apr 22, 2025

Since the LSP file URL requires an absolute path, ensure that all include files use absolute paths

@llvmbot
Copy link
Member

llvmbot commented Apr 22, 2025

@llvm/pr-subscribers-llvm-support

Author: tcwzxx (tcwzxx)

Changes

Since the LSP file URL requires an absolute path, ensure that all include files use absolute paths


Full diff: https://github.com/llvm/llvm-project/pull/136687.diff

1 Files Affected:

  • (modified) llvm/lib/Support/SourceMgr.cpp (+6-2)
diff --git a/llvm/lib/Support/SourceMgr.cpp b/llvm/lib/Support/SourceMgr.cpp
index 3f97213d86c05..1a6389737c1fb 100644
--- a/llvm/lib/Support/SourceMgr.cpp
+++ b/llvm/lib/Support/SourceMgr.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Locale.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -52,15 +53,18 @@ unsigned SourceMgr::AddIncludeFile(const std::string &Filename,
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 SourceMgr::OpenIncludeFile(const std::string &Filename,
                            std::string &IncludedFile) {
+  SmallString<128> Path{Filename};
+  sys::fs::make_absolute(Path);
   ErrorOr<std::unique_ptr<MemoryBuffer>> NewBufOrErr =
-      MemoryBuffer::getFile(Filename);
+      MemoryBuffer::getFile(Path);
 
-  SmallString<64> Buffer(Filename);
+  SmallString<64> Buffer(Path);
   // If the file didn't exist directly, see if it's in an include path.
   for (unsigned i = 0, e = IncludeDirectories.size(); i != e && !NewBufOrErr;
        ++i) {
     Buffer = IncludeDirectories[i];
     sys::path::append(Buffer, Filename);
+    sys::fs::make_absolute(Buffer);
     NewBufOrErr = MemoryBuffer::getFile(Buffer);
   }
 

@tcwzxx tcwzxx requested a review from zero9178 April 22, 2025 11:58
@zero9178 zero9178 requested a review from River707 April 22, 2025 18:56
@zero9178
Copy link
Member

Not super familiar with the LSP spec unfortunately, but assuming there is no better place to convert to absolute paths it makes sense.

Could this be tested in some way? E.g. that a URI now contains more components of a path that it previously didn't

@tcwzxx
Copy link
Member Author

tcwzxx commented Apr 23, 2025

Thanks,

but assuming there is no better place

But after a file is opened, transforming the file path is problematic if the cwd has changed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants