Skip to content

Commit 58d0ee0

Browse files
author
Nathan Hawes
committed
[ParseableInterface] Distinguish SDK and non-SDK dependencies
This allows the SDK to be relocated without automatically resulting in a rebuild. Based on an old patch from Jordan Rose.
1 parent f683373 commit 58d0ee0

File tree

9 files changed

+222
-28
lines changed

9 files changed

+222
-28
lines changed

include/swift/Serialization/ModuleFormat.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
5252
/// describe what change you made. The content of this comment isn't important;
5353
/// it just ensures a conflict if two people change the module format.
5454
/// Don't worry about adhering to the 80-column limit for this line.
55-
const uint16_t SWIFTMODULE_VERSION_MINOR = 483; // Remove default arg expansion
55+
const uint16_t SWIFTMODULE_VERSION_MINOR = 484; // SDK-relative dependencies flag
5656

5757
using DeclIDField = BCFixed<31>;
5858

@@ -687,6 +687,7 @@ namespace input_block {
687687
FileSizeField, // file size (for validation)
688688
FileModTimeOrContentHashField, // mtime or content hash (for validation)
689689
BCFixed<1>, // are we reading mtime (0) or hash (1)?
690+
BCFixed<1>, // SDK-relative?
690691
BCBlob // path
691692
>;
692693
}

include/swift/Serialization/SerializationOptions.h

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,14 @@ namespace swift {
3939
/// appropriate strategy for how to verify if it's up-to-date.
4040
class FileDependency {
4141
/// The size of the file on disk, in bytes.
42-
uint64_t Size : 63;
42+
uint64_t Size : 62;
4343

4444
/// A dependency can be either hash-based or modification-time-based.
4545
bool IsHashBased : 1;
4646

47+
/// The dependency path can be absolute or relative to the SDK
48+
bool IsSDKRelative : 1;
49+
4750
union {
4851
/// The last modification time of the file.
4952
uint64_t ModificationTime;
@@ -56,22 +59,22 @@ namespace swift {
5659
std::string Path;
5760

5861
FileDependency(uint64_t size, bool isHash, uint64_t hashOrModTime,
59-
StringRef path):
60-
Size(size), IsHashBased(isHash), ModificationTime(hashOrModTime),
61-
Path(path) {}
62+
StringRef path, bool isSDKRelative):
63+
Size(size), IsHashBased(isHash), IsSDKRelative(isSDKRelative),
64+
ModificationTime(hashOrModTime), Path(path) {}
6265
public:
6366
FileDependency() = delete;
6467

6568
/// Creates a new hash-based file dependency.
6669
static FileDependency
67-
hashBased(StringRef path, uint64_t size, uint64_t hash) {
68-
return FileDependency(size, /*isHash*/true, hash, path);
70+
hashBased(StringRef path, bool isSDKRelative, uint64_t size, uint64_t hash) {
71+
return FileDependency(size, /*isHash*/true, hash, path, isSDKRelative);
6972
}
7073

7174
/// Creates a new modification time-based file dependency.
7275
static FileDependency
73-
modTimeBased(StringRef path, uint64_t size, uint64_t mtime) {
74-
return FileDependency(size, /*isHash*/false, mtime, path);
76+
modTimeBased(StringRef path, bool isSDKRelative, uint64_t size, uint64_t mtime) {
77+
return FileDependency(size, /*isHash*/false, mtime, path, isSDKRelative);
7578
}
7679

7780
/// Updates the last-modified time of this dependency.
@@ -94,6 +97,9 @@ namespace swift {
9497
/// based on content hash.
9598
bool isHashBased() const { return IsHashBased; }
9699

100+
/// Determines if this dependency is absolute or relative to the SDK.
101+
bool isSDKRelative() const { return IsSDKRelative; }
102+
97103
/// Determines if this dependency is hash-based and should be validated
98104
/// based on modification time.
99105
bool isModificationTimeBased() const { return !IsHashBased; }

lib/AST/ModuleLoader.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,8 @@ void
3434
DependencyTracker::addDependency(StringRef File, bool IsSystem) {
3535
// DependencyTracker exposes an interface that (intentionally) does not talk
3636
// about clang at all, nor about missing deps. It does expose an IsSystem
37-
// dimension, though it is presently always false, we accept it and pass it
38-
// along to the clang DependencyCollector in case Swift callers start setting
39-
// it to true someday.
37+
// dimension, which we accept and pass along to the clang DependencyCollector.
38+
// along to the clang DependencyCollector.
4039
clangCollector->maybeAddDependency(File, /*FromModule=*/false,
4140
IsSystem, /*IsModuleFile=*/false,
4241
/*IsMissing=*/false);

lib/Frontend/ParseableInterfaceModuleLoader.cpp

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,8 @@ class swift::ParseableInterfaceBuilder {
368368
bool collectDepsForSerialization(CompilerInstance &SubInstance,
369369
SmallVectorImpl<FileDependency> &Deps,
370370
bool IsHashBased) {
371+
StringRef SDKPath = SubInstance.getASTContext().SearchPathOpts.SDKPath;
372+
371373
auto DTDeps = SubInstance.getDependencyTracker()->getDependencies();
372374
SmallVector<StringRef, 16> InitialDepNames(DTDeps.begin(), DTDeps.end());
373375
InitialDepNames.push_back(interfacePath);
@@ -396,17 +398,39 @@ class swift::ParseableInterfaceBuilder {
396398
if (!Status)
397399
return true;
398400

401+
bool IsSDKRelative = false;
402+
StringRef DepNameToStore = DepName;
403+
if (SDKPath.size() > 1 && DepName.startswith(SDKPath)) {
404+
assert(DepName.size() > SDKPath.size() &&
405+
"should never depend on a directory");
406+
if (llvm::sys::path::is_separator(DepName[SDKPath.size()])) {
407+
// Is the DepName something like ${SDKPath}/foo.h"?
408+
DepNameToStore = DepName.substr(SDKPath.size() + 1);
409+
IsSDKRelative = true;
410+
} else if (llvm::sys::path::is_separator(SDKPath.back())) {
411+
// Is the DepName something like "${SDKPath}foo.h", where SDKPath
412+
// itself contains a trailing slash?
413+
DepNameToStore = DepName.substr(SDKPath.size());
414+
IsSDKRelative = true;
415+
} else {
416+
// We have something next to an SDK, like "Foo.sdk.h", that's somehow
417+
// become a dependency.
418+
}
419+
}
420+
399421
if (IsHashBased) {
400422
auto buf = getDepBuf();
401423
if (!buf) return true;
402424
uint64_t hash = xxHash64(buf->getBuffer());
403425
Deps.push_back(
404-
FileDependency::hashBased(DepName, Status->getSize(), hash));
426+
FileDependency::hashBased(DepNameToStore, IsSDKRelative,
427+
Status->getSize(), hash));
405428
} else {
406429
uint64_t mtime =
407430
Status->getLastModificationTime().time_since_epoch().count();
408431
Deps.push_back(
409-
FileDependency::modTimeBased(DepName, Status->getSize(), mtime));
432+
FileDependency::modTimeBased(DepNameToStore, IsSDKRelative,
433+
Status->getSize(), mtime));
410434
}
411435

412436
if (moduleCachePath.empty())
@@ -681,10 +705,23 @@ class ParseableInterfaceModuleLoaderImpl {
681705
OutPath.append(OutExt);
682706
}
683707

708+
/// Constructs the full path of the dependency \p dep by prepending the SDK
709+
/// path if necessary.
710+
StringRef getFullDependencyPath(const FileDependency &dep,
711+
SmallVectorImpl<char> &scratch) const {
712+
if (!dep.isSDKRelative())
713+
return dep.getPath();
714+
715+
StringRef SDKPath = ctx.SearchPathOpts.SDKPath;
716+
scratch.assign(SDKPath.begin(), SDKPath.end());
717+
llvm::sys::path::append(scratch, dep.getPath());
718+
return StringRef(scratch.data(), scratch.size());
719+
}
720+
684721
// Checks that a dependency read from the cached module is up to date compared
685722
// to the interface file it represents.
686-
bool dependencyIsUpToDate(const FileDependency &dep) {
687-
auto status = getStatusOfDependency(fs, dep.getPath(), interfacePath,
723+
bool dependencyIsUpToDate(const FileDependency &dep, StringRef fullPath) {
724+
auto status = getStatusOfDependency(fs, fullPath, interfacePath,
688725
diags, diagnosticLoc);
689726
if (!status) return false;
690727

@@ -701,7 +738,7 @@ class ParseableInterfaceModuleLoaderImpl {
701738

702739
// Slow path: if the dependency is verified by content hash, check it vs.
703740
// the hash of the file.
704-
auto buf = getBufferOfDependency(fs, dep.getPath(), interfacePath,
741+
auto buf = getBufferOfDependency(fs, fullPath, interfacePath,
705742
diags, diagnosticLoc);
706743
if (!buf) return false;
707744

@@ -711,10 +748,12 @@ class ParseableInterfaceModuleLoaderImpl {
711748
// Check if all the provided file dependencies are up-to-date compared to
712749
// what's currently on disk.
713750
bool dependenciesAreUpToDate(ArrayRef<FileDependency> deps) {
751+
SmallString<128> SDKRelativeBuffer;
714752
for (auto &in : deps) {
753+
StringRef fullPath = getFullDependencyPath(in, SDKRelativeBuffer);
715754
if (dependencyTracker)
716-
dependencyTracker->addDependency(in.getPath(), /*isSystem*/false);
717-
if (!dependencyIsUpToDate(in)) {
755+
dependencyTracker->addDependency(fullPath, /*isSystem*/in.isSDKRelative());
756+
if (!dependencyIsUpToDate(in, fullPath)) {
718757
LLVM_DEBUG(llvm::dbgs() << "Dep " << in.getPath()
719758
<< " is directly out of date\n");
720759
return false;
@@ -763,9 +802,12 @@ class ParseableInterfaceModuleLoaderImpl {
763802

764803
// Next, check the dependencies in the forwarding file.
765804
for (auto &dep : fwd.dependencies) {
805+
// Forwarding modules expand SDKRelative paths when generated, so are
806+
// guaranteed to be absolute.
766807
deps.push_back(
767808
FileDependency::modTimeBased(
768-
dep.path, dep.size, dep.lastModificationTime));
809+
dep.path, /*isSDKRelative=*/false, dep.size,
810+
dep.lastModificationTime));
769811
}
770812
if (!dependenciesAreUpToDate(deps))
771813
return false;
@@ -919,8 +961,9 @@ class ParseableInterfaceModuleLoaderImpl {
919961
addDependency(fwd.underlyingModulePath);
920962

921963
// Add all the dependencies from the prebuilt module.
964+
SmallString<128> SDKRelativeBuffer;
922965
for (auto dep : deps) {
923-
addDependency(dep.getPath());
966+
addDependency(getFullDependencyPath(dep, SDKRelativeBuffer));
924967
}
925968

926969
return withOutputFile(diags, outputPath,
@@ -937,6 +980,7 @@ class ParseableInterfaceModuleLoaderImpl {
937980
/// loading strategy.
938981
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
939982
findOrBuildLoadableModule() {
983+
940984
// Track system dependencies if the parent tracker is set to do so.
941985
// FIXME: This means -track-system-dependencies isn't honored when the
942986
// top-level invocation isn't tracking dependencies
@@ -1057,11 +1101,10 @@ bool ParseableInterfaceModuleLoader::buildSwiftModuleFromSwiftInterface(
10571101
bool SerializeDependencyHashes, bool TrackSystemDependencies) {
10581102
ParseableInterfaceBuilder builder(Ctx, InPath, ModuleName,
10591103
CacheDir, PrebuiltCacheDir,
1060-
SerializeDependencyHashes);
1061-
// FIXME: We really only want to serialize 'important' dependencies here, and
1062-
// make them relocatable (SDK-relative) if we want to ship the built
1063-
// swiftmodules to another machine. Just track them as absolute paths
1064-
// for now, so we can test the dependency tracking locally.
1104+
SerializeDependencyHashes,
1105+
TrackSystemDependencies);
1106+
// FIXME: We really only want to serialize 'important' dependencies here, if
1107+
// we want to ship the built swiftmodules to another machine.
10651108
return builder.buildSwiftModule(OutPath, /*shouldSerializeDeps*/true,
10661109
/*ModuleBuffer*/nullptr);
10671110
}

lib/Serialization/ModuleFile.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,14 +253,16 @@ static bool validateInputBlock(
253253
switch (kind) {
254254
case input_block::FILE_DEPENDENCY: {
255255
bool isHashBased = scratch[2] != 0;
256+
bool isSDKRelative = scratch[3] != 0;
257+
256258
if (isHashBased) {
257259
dependencies.push_back(
258260
SerializationOptions::FileDependency::hashBased(
259-
blobData, scratch[0], scratch[1]));
261+
blobData, isSDKRelative, scratch[0], scratch[1]));
260262
} else {
261263
dependencies.push_back(
262264
SerializationOptions::FileDependency::modTimeBased(
263-
blobData, scratch[0], scratch[1]));
265+
blobData, isSDKRelative, scratch[0], scratch[1]));
264266
}
265267
break;
266268
}

lib/Serialization/Serialization.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,6 +1076,7 @@ void Serializer::writeInputBlock(const SerializationOptions &options) {
10761076
dep.getSize(),
10771077
getRawModTimeOrHash(dep),
10781078
dep.isHashBased(),
1079+
dep.isSDKRelative(),
10791080
dep.getPath());
10801081
}
10811082

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -parse-stdlib -module-name ExportedLib
3+
4+
@_exported import SomeCModule
5+
6+
public struct ExportedInterface {
7+
@inlinable public init() {}
8+
}
9+
public var testValue: ExportedInterface {
10+
@inlinable get { return ExportedInterface() }
11+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -parse-stdlib -module-name SdkLib
3+
4+
@_exported import ExportedLib
5+
6+
public var testValue2: ExportedInterface {
7+
@inlinable get { return ExportedInterface() }
8+
}

0 commit comments

Comments
 (0)