Skip to content

Commit 02e422e

Browse files
author
Nathan Hawes
committed
[ParseableInterface] Normalize paths before comparison when serializing dependencies
Dependencies in the SDK have their paths serialized relative to it to allow the produced swift module to stay valid when the SDK moves. In the windows build mixed slashes were coming through in these paths and breaking the check for whether a dependency was in the SDK or not. This patch ensures both paths are using native path separators prior to the comparison to hopefuly fix the Windows build.
1 parent 1131fe1 commit 02e422e

File tree

1 file changed

+58
-41
lines changed

1 file changed

+58
-41
lines changed

Diff for: lib/Frontend/ParseableInterfaceModuleLoader.cpp

+58-41
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,32 @@ static Optional<llvm::vfs::Status> getStatusOfDependency(
247247
return status.get();
248248
}
249249

250+
/// If the file dependency in \p FullDepPath is inside the \p Base directory,
251+
/// this returns its path relative to \p Base. Otherwise it returns None.
252+
static Optional<StringRef> getRelativeDepPath(StringRef DepPath,
253+
StringRef Base) {
254+
// If Base is the root directory, or DepPath does not start with Base, bail.
255+
if (Base.size() <= 1 || !DepPath.startswith(Base)) {
256+
return None;
257+
}
258+
259+
assert(DepPath.size() > Base.size() &&
260+
"should never depend on a directory");
261+
262+
// Is the DepName something like ${Base}/foo.h"?
263+
if (path::is_separator(DepPath[Base.size()]))
264+
return DepPath.substr(Base.size() + 1);
265+
266+
// Is the DepName something like "${Base}foo.h", where Base
267+
// itself contains a trailing slash?
268+
if (path::is_separator(Base.back()))
269+
return DepPath.substr(Base.size());
270+
271+
// We have something next to Base, like "Base.h", that's somehow
272+
// become a dependency.
273+
return None;
274+
}
275+
250276
#pragma mark - Module Building
251277

252278
/// Builds a parseable module interface into a .swiftmodule at the provided
@@ -365,52 +391,29 @@ class swift::ParseableInterfaceBuilder {
365391
bool collectDepsForSerialization(CompilerInstance &SubInstance,
366392
SmallVectorImpl<FileDependency> &Deps,
367393
bool IsHashBased) {
368-
StringRef SDKPath = SubInstance.getASTContext().SearchPathOpts.SDKPath;
369-
StringRef ResourcePath =
370-
SubInstance.getASTContext().SearchPathOpts.RuntimeResourcePath;
394+
auto &Opts = SubInstance.getASTContext().SearchPathOpts;
395+
SmallString<128> SDKPath(Opts.SDKPath);
396+
path::native(SDKPath);
397+
SmallString<128> ResourcePath(Opts.RuntimeResourcePath);
398+
path::native(ResourcePath);
399+
371400
auto DTDeps = SubInstance.getDependencyTracker()->getDependencies();
372401
SmallVector<StringRef, 16> InitialDepNames(DTDeps.begin(), DTDeps.end());
373402
InitialDepNames.push_back(interfacePath);
374403
llvm::StringSet<> AllDepNames;
404+
SmallString<128> Scratch;
405+
406+
for (const auto &InitialDepName : InitialDepNames) {
407+
path::native(InitialDepName, Scratch);
408+
StringRef DepName = Scratch.str();
375409

376-
for (auto const &DepName : InitialDepNames) {
377410
assert(moduleCachePath.empty() || !DepName.startswith(moduleCachePath));
378411
assert(prebuiltCachePath.empty() || !DepName.startswith(prebuiltCachePath));
379412

380-
/// Lazily load the dependency buffer if we need it. If we're not
381-
/// dealing with a hash-based dependencies, and if the dependency is
382-
/// not a .swiftmodule, we can avoid opening the buffer.
383-
std::unique_ptr<llvm::MemoryBuffer> DepBuf = nullptr;
384-
auto getDepBuf = [&]() -> llvm::MemoryBuffer * {
385-
if (DepBuf) return DepBuf.get();
386-
if (auto Buf = getBufferOfDependency(fs, DepName, interfacePath,
387-
diags, diagnosticLoc)) {
388-
DepBuf = std::move(Buf);
389-
return DepBuf.get();
390-
}
391-
return nullptr;
392-
};
393-
394-
// Adjust the paths of dependences in the SDK to be relative to it
395-
bool IsSDKRelative = false;
396-
StringRef DepNameToStore = DepName;
397-
if (SDKPath.size() > 1 && DepName.startswith(SDKPath)) {
398-
assert(DepName.size() > SDKPath.size() &&
399-
"should never depend on a directory");
400-
if (llvm::sys::path::is_separator(DepName[SDKPath.size()])) {
401-
// Is the DepName something like ${SDKPath}/foo.h"?
402-
DepNameToStore = DepName.substr(SDKPath.size() + 1);
403-
IsSDKRelative = true;
404-
} else if (llvm::sys::path::is_separator(SDKPath.back())) {
405-
// Is the DepName something like "${SDKPath}foo.h", where SDKPath
406-
// itself contains a trailing slash?
407-
DepNameToStore = DepName.substr(SDKPath.size());
408-
IsSDKRelative = true;
409-
} else {
410-
// We have something next to an SDK, like "Foo.sdk.h", that's somehow
411-
// become a dependency.
412-
}
413-
}
413+
// Serialize the paths of dependencies in the SDK relative to it.
414+
Optional<StringRef> SDKRelativePath = getRelativeDepPath(DepName, SDKPath);
415+
StringRef DepNameToStore = SDKRelativePath.getValueOr(DepName);
416+
bool IsSDKRelative = SDKRelativePath.hasValue();
414417

415418
if (AllDepNames.insert(DepName).second && dependencyTracker) {
416419
dependencyTracker->addDependency(DepName, /*isSystem*/IsSDKRelative);
@@ -425,6 +428,20 @@ class swift::ParseableInterfaceBuilder {
425428
if (!Status)
426429
return true;
427430

431+
/// Lazily load the dependency buffer if we need it. If we're not
432+
/// dealing with a hash-based dependencies, and if the dependency is
433+
/// not a .swiftmodule, we can avoid opening the buffer.
434+
std::unique_ptr<llvm::MemoryBuffer> DepBuf = nullptr;
435+
auto getDepBuf = [&]() -> llvm::MemoryBuffer * {
436+
if (DepBuf) return DepBuf.get();
437+
if (auto Buf = getBufferOfDependency(fs, DepName, interfacePath,
438+
diags, diagnosticLoc)) {
439+
DepBuf = std::move(Buf);
440+
return DepBuf.get();
441+
}
442+
return nullptr;
443+
};
444+
428445
if (IsHashBased) {
429446
auto buf = getDepBuf();
430447
if (!buf) return true;
@@ -687,8 +704,7 @@ class ParseableInterfaceModuleLoaderImpl {
687704
if (!dep.isSDKRelative())
688705
return dep.getPath();
689706

690-
StringRef SDKPath = ctx.SearchPathOpts.SDKPath;
691-
scratch.assign(SDKPath.begin(), SDKPath.end());
707+
path::native(ctx.SearchPathOpts.SDKPath, scratch);
692708
llvm::sys::path::append(scratch, dep.getPath());
693709
return StringRef(scratch.data(), scratch.size());
694710
}
@@ -1003,7 +1019,8 @@ class ParseableInterfaceModuleLoaderImpl {
10031019
SmallString<128> SDKRelativeBuffer;
10041020
for (auto &dep: allDeps) {
10051021
StringRef fullPath = getFullDependencyPath(dep, SDKRelativeBuffer);
1006-
dependencyTracker->addDependency(fullPath, dep.isSDKRelative());
1022+
dependencyTracker->addDependency(fullPath,
1023+
/*IsSystem=*/dep.isSDKRelative());
10071024
}
10081025
}
10091026

0 commit comments

Comments
 (0)