Skip to content

Commit 2a702cd

Browse files
authored
Driver: Avoid llvm::sys::path::append if resource directory absolute.
After #145996 CLANG_RESOURCE_DIR can be an absolute path so we need to handle it correctly in the driver. llvm::sys::path::append does not append absolute paths in the way that I expected (or consistent with other similar APIs such as C++17 std::filesystem::path::append or Python os.path.join); instead, it effectively discards the leading / and appends the resulting relative path (e.g. append(P, "/bar") with P = "/foo" sets P to "/foo/bar"). Many tests start failing if I try to align llvm::sys::path::append with the other APIs because of callers that expect the existing behavior, so for now let's add a special case here for absolute resource paths, and document the behavior in Path.h. Reviewers: MaskRay Reviewed By: MaskRay Pull Request: #146449
1 parent aa1d9a4 commit 2a702cd

File tree

2 files changed

+8
-1
lines changed

2 files changed

+8
-1
lines changed

clang/lib/Driver/Driver.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,12 @@ std::string Driver::GetResourcesPath(StringRef BinaryPath) {
192192

193193
StringRef ConfiguredResourceDir(CLANG_RESOURCE_DIR);
194194
if (!ConfiguredResourceDir.empty()) {
195-
llvm::sys::path::append(P, ConfiguredResourceDir);
195+
// FIXME: We should fix the behavior of llvm::sys::path::append so we don't
196+
// need to check for absolute paths here.
197+
if (llvm::sys::path::is_absolute(ConfiguredResourceDir))
198+
P = ConfiguredResourceDir;
199+
else
200+
llvm::sys::path::append(P, ConfiguredResourceDir);
196201
} else {
197202
// On Windows, libclang.dll is in bin/.
198203
// On non-Windows, libclang.so/.dylib is in lib/.

llvm/include/llvm/Support/Path.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,8 @@ LLVM_ABI bool remove_dots(SmallVectorImpl<char> &path,
223223
/// /foo + bar/f => /foo/bar/f
224224
/// /foo/ + bar/f => /foo/bar/f
225225
/// foo + bar/f => foo/bar/f
226+
/// foo + /bar/f => foo/bar/f (FIXME: will be changed to /bar/f to align
227+
/// with C++17 std::filesystem::path::append)
226228
/// @endcode
227229
///
228230
/// @param path Set to \a path + \a component.

0 commit comments

Comments
 (0)