Skip to content

Commit 98ebfeb

Browse files
varungandhi-applevarungandhi-apple
authored
Merge pull request swiftlang#26243 from varungandhi-apple/vg-add-direct-imports-to-trace
Add information on direct imports and library evolution to trace file.
2 parents 54a99eb + 0205a10 commit 98ebfeb

8 files changed

+221
-49
lines changed

‎lib/FrontendTool/FrontendTool.cpp

Lines changed: 111 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
#include <deque>
8686
#include <memory>
8787
#include <unordered_set>
88+
#include <utility>
8889

8990
#if !defined(_MSC_VER) && !defined(__MINGW32__)
9091
#include <unistd.h>
@@ -184,73 +185,152 @@ static bool emitMakeDependenciesIfNeeded(DiagnosticEngine &diags,
184185
}
185186

186187
namespace {
188+
struct SwiftModuleTraceInfo {
189+
Identifier Name;
190+
std::string Path;
191+
bool IsImportedDirectly;
192+
bool SupportsLibraryEvolution;
193+
};
194+
187195
struct LoadedModuleTraceFormat {
188-
std::string Name;
196+
static const unsigned CurrentVersion = 2;
197+
unsigned Version;
198+
Identifier Name;
189199
std::string Arch;
190-
std::vector<std::string> SwiftModules;
200+
std::vector<SwiftModuleTraceInfo> SwiftModules;
191201
};
192202
}
193203

194204
namespace swift {
195205
namespace json {
206+
template <> struct ObjectTraits<SwiftModuleTraceInfo> {
207+
static void mapping(Output &out, SwiftModuleTraceInfo &contents) {
208+
StringRef name = contents.Name.str();
209+
out.mapRequired("name", name);
210+
out.mapRequired("path", contents.Path);
211+
out.mapRequired("isImportedDirectly", contents.IsImportedDirectly);
212+
out.mapRequired("supportsLibraryEvolution",
213+
contents.SupportsLibraryEvolution);
214+
}
215+
};
216+
217+
// Version notes:
218+
// 1. Keys: name, arch, swiftmodules
219+
// 2. New keys: version, swiftmodulesDetailedInfo
196220
template <> struct ObjectTraits<LoadedModuleTraceFormat> {
197221
static void mapping(Output &out, LoadedModuleTraceFormat &contents) {
198-
out.mapRequired("name", contents.Name);
222+
out.mapRequired("version", contents.Version);
223+
224+
StringRef name = contents.Name.str();
225+
out.mapRequired("name", name);
226+
199227
out.mapRequired("arch", contents.Arch);
200-
out.mapRequired("swiftmodules", contents.SwiftModules);
228+
229+
// The 'swiftmodules' key is kept for backwards compatibility.
230+
std::vector<std::string> moduleNames;
231+
for (auto &m : contents.SwiftModules)
232+
moduleNames.push_back(m.Path);
233+
out.mapRequired("swiftmodules", moduleNames);
234+
235+
out.mapRequired("swiftmodulesDetailedInfo", contents.SwiftModules);
201236
}
202237
};
203238
}
204239
}
205240

206-
static bool emitLoadedModuleTraceIfNeeded(ASTContext &ctxt,
241+
static bool emitLoadedModuleTraceIfNeeded(ModuleDecl *mainModule,
207242
DependencyTracker *depTracker,
208-
StringRef loadedModuleTracePath,
209-
StringRef moduleName) {
243+
StringRef loadedModuleTracePath) {
210244
if (loadedModuleTracePath.empty())
211245
return false;
212246
std::error_code EC;
213247
llvm::raw_fd_ostream out(loadedModuleTracePath, EC, llvm::sys::fs::F_Append);
214248

249+
ASTContext &ctxt = mainModule->getASTContext();
215250
if (out.has_error() || EC) {
216251
ctxt.Diags.diagnose(SourceLoc(), diag::error_opening_output,
217252
loadedModuleTracePath, EC.message());
218253
out.clear_error();
219254
return true;
220255
}
221256

222-
llvm::SmallVector<std::string, 16> swiftModules;
257+
ModuleDecl::ImportFilter filter = ModuleDecl::ImportFilterKind::Public;
258+
filter |= ModuleDecl::ImportFilterKind::Private;
259+
filter |= ModuleDecl::ImportFilterKind::ImplementationOnly;
260+
SmallVector<ModuleDecl::ImportedModule, 8> imports;
261+
mainModule->getImportedModules(imports, filter);
223262

224-
// Canonicalise all the paths by opening them.
225-
for (auto &dep : depTracker->getDependencies()) {
226-
llvm::SmallString<256> buffer;
227-
StringRef realPath;
228-
int FD;
263+
SmallPtrSet<ModuleDecl *, 8> importedModules;
264+
for (std::pair<ModuleDecl::AccessPathTy, ModuleDecl *> &import : imports)
265+
importedModules.insert(import.second);
266+
267+
llvm::DenseMap<StringRef, ModuleDecl *> pathToModuleDecl;
268+
for (auto &module : ctxt.LoadedModules) {
269+
ModuleDecl *loadedDecl = module.second;
270+
assert(loadedDecl && "Expected loaded module to be non-null.");
271+
assert(!loadedDecl->getModuleFilename().empty()
272+
&& "Don't know how to handle modules with empty names.");
273+
pathToModuleDecl.insert(
274+
std::make_pair(loadedDecl->getModuleFilename(), loadedDecl));
275+
}
276+
277+
std::vector<SwiftModuleTraceInfo> swiftModules;
278+
SmallString<256> buffer;
279+
for (auto &depPath : depTracker->getDependencies()) {
280+
StringRef realDepPath;
229281
// FIXME: appropriate error handling
230-
if (llvm::sys::fs::openFileForRead(dep, FD, llvm::sys::fs::OF_None,
231-
&buffer)) {
232-
// Couldn't open the file now, so let's just assume the old path was
282+
if (llvm::sys::fs::real_path(depPath, buffer,/*expand_tilde=*/true))
283+
// Couldn't find the canonical path, so let's just assume the old one was
233284
// canonical (enough).
234-
realPath = dep;
235-
} else {
236-
realPath = buffer.str();
237-
// Not much we can do about failing to close.
238-
(void)close(FD);
239-
}
285+
realDepPath = depPath;
286+
else
287+
realDepPath = buffer.str();
240288

241289
// Decide if this is a swiftmodule based on the extension of the raw
242290
// dependency path, as the true file may have a different one.
243-
auto ext = llvm::sys::path::extension(dep);
244-
if (file_types::lookupTypeForExtension(ext) ==
245-
file_types::TY_SwiftModuleFile) {
246-
swiftModules.push_back(realPath);
291+
// For example, this might happen when the canonicalized path points to
292+
// a Content Addressed Storage (CAS) location.
293+
auto moduleFileType =
294+
file_types::lookupTypeForExtension(llvm::sys::path::extension(depPath));
295+
if (moduleFileType == file_types::TY_SwiftModuleFile
296+
|| moduleFileType == file_types::TY_SwiftParseableInterfaceFile) {
297+
auto dep = pathToModuleDecl.find(depPath);
298+
assert(dep != pathToModuleDecl.end()
299+
&& "Dependency must've been loaded.");
300+
ModuleDecl *depMod = dep->second;
301+
swiftModules.push_back({
302+
/*Name=*/
303+
depMod->getName(),
304+
/*Path=*/
305+
realDepPath,
306+
// TODO: There is an edge case which is not handled here.
307+
// When we build a framework using -import-underlying-module, or an
308+
// app/test using -import-objc-header, we should look at the direct
309+
// imports of the bridging modules, and mark those as our direct
310+
// imports.
311+
/*IsImportedDirectly=*/
312+
importedModules.find(depMod) != importedModules.end(),
313+
/*SupportsLibraryEvolution=*/
314+
depMod->isResilient()
315+
});
247316
}
248317
}
249318

319+
// Almost a re-implementation of reversePathSortedFilenames :(.
320+
std::sort(
321+
swiftModules.begin(), swiftModules.end(),
322+
[](const SwiftModuleTraceInfo &m1, const SwiftModuleTraceInfo &m2) -> bool {
323+
return std::lexicographical_compare(
324+
m1.Path.rbegin(), m1.Path.rend(),
325+
m2.Path.rbegin(), m2.Path.rend());
326+
});
327+
250328
LoadedModuleTraceFormat trace = {
251-
/*name=*/moduleName,
329+
/*version=*/LoadedModuleTraceFormat::CurrentVersion,
330+
/*name=*/mainModule->getName(),
252331
/*arch=*/ctxt.LangOpts.Target.getArchName(),
253-
/*swiftmodules=*/reversePathSortedFilenames(swiftModules)};
332+
swiftModules
333+
};
254334

255335
// raw_fd_ostream is unbuffered, and we may have multiple processes writing,
256336
// so first write the whole thing into memory and dump out that buffer to the
@@ -270,13 +350,13 @@ static bool emitLoadedModuleTraceIfNeeded(ASTContext &ctxt,
270350
}
271351

272352
static bool
273-
emitLoadedModuleTraceForAllPrimariesIfNeeded(ASTContext &ctxt,
353+
emitLoadedModuleTraceForAllPrimariesIfNeeded(ModuleDecl *mainModule,
274354
DependencyTracker *depTracker,
275355
const FrontendOptions &opts) {
276356
return opts.InputsAndOutputs.forEachInputProducingSupplementaryOutput(
277357
[&](const InputFile &input) -> bool {
278358
return emitLoadedModuleTraceIfNeeded(
279-
ctxt, depTracker, input.loadedModuleTracePath(), opts.ModuleName);
359+
mainModule, depTracker, input.loadedModuleTracePath());
280360
});
281361
}
282362

@@ -1007,7 +1087,7 @@ static bool performCompile(CompilerInstance &Instance,
10071087
emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Invocation, Instance);
10081088

10091089
(void)emitLoadedModuleTraceForAllPrimariesIfNeeded(
1010-
Context, Instance.getDependencyTracker(), opts);
1090+
Instance.getMainModule(), Instance.getDependencyTracker(), opts);
10111091

10121092
if (Context.hadError()) {
10131093
// Emit the index store data even if there were compiler errors.

‎test/Driver/loaded_module_trace.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,28 @@
66
// RUN: %FileCheck -check-prefix=CHECK-CONFIRM-ONELINE %s < %t/loaded_module_trace.trace.json
77

88
// CHECK: {
9+
// CHECK: "version":2
910
// CHECK: "name":"loaded_module_trace"
1011
// CHECK: "arch":"{{[^"]*}}"
1112
// CHECK: "swiftmodules":[
1213
// CHECK-DAG: "{{[^"]*\\[/\\]}}Module2.swiftmodule"
1314
// CHECK-DAG: "{{[^"]*\\[/\\]}}Swift.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
1415
// CHECK-DAG: "{{[^"]*\\[/\\]}}SwiftOnoneSupport.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
16+
// CHECK-DAG: "{{[^"]*\\[/\\]}}Module.swiftmodule"
17+
// CHECK: ]
18+
19+
// The checks are compressed into one line as the order of the objects varies
20+
// between OSes due to change in filepaths.
21+
22+
// CHECK: "swiftmodulesDetailedInfo":[
23+
// CHECK-DAG: {"name":"Module2","path":"{{[^"]*\\[/\\]}}Module2.swiftmodule","isImportedDirectly":true,"supportsLibraryEvolution":false}
24+
// CHECK-DAG: {"name":"Swift","path":"{{[^"]*\\[/\\]}}Swift.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}","isImportedDirectly":true,"supportsLibraryEvolution":true}
25+
// CHECK-DAG: {"name":"SwiftOnoneSupport","path":"{{[^"]*\\[/\\]}}SwiftOnoneSupport.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}","isImportedDirectly":true,"supportsLibraryEvolution":true}
26+
// CHECK-DAG: {"name":"Module","path":"{{[^"]*\\[/\\]}}Module.swiftmodule","isImportedDirectly":false,"supportsLibraryEvolution":false}
1527
// CHECK: ]
1628
// CHECK: }
1729

1830
// Make sure it's all on a single line.
1931
// CHECK-CONFIRM-ONELINE: {"name":{{.*}}]}
2032

21-
// 'Module2' imports 'Module', so we're also checking we don't get transitive dependencies.
2233
import Module2

‎test/Driver/loaded_module_trace_append.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,5 @@
33
// RUN: env SWIFT_LOADED_MODULE_TRACE_FILE=%t %target-build-swift -module-name loaded_module_trace_append2 -c %s -o- > /dev/null
44
// RUN: %FileCheck %s < %t
55

6-
// CHECK: {"name":"loaded_module_trace_append",{{.*}}]}
7-
// CHECK-NEXT: {"name":"loaded_module_trace_append2",{{.*}}]}
6+
// CHECK: {{{.*}},"name":"loaded_module_trace_append",{{.*}}]}
7+
// CHECK-NEXT: {{{.*}},"name":"loaded_module_trace_append2",{{.*}}]}

‎test/Driver/loaded_module_trace_env.swift

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,24 @@
33
// RUN: %FileCheck %s < %t
44

55
// CHECK: {
6+
// CHECK: "version":2
67
// CHECK: "name":"loaded_module_trace_env"
78
// CHECK: "arch":"{{[^"]*}}"
89
// CHECK: "swiftmodules":[
9-
// CHECK: "{{[^"]*\\[/\\]}}Swift.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
10-
// CHECK: "{{[^"]*\\[/\\]}}SwiftOnoneSupport.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
10+
// CHECK-DAG: "{{[^"]*\\[/\\]}}Swift.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
11+
// CHECK-DAG: "{{[^"]*\\[/\\]}}SwiftOnoneSupport.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
1112
// CHECK: ]
13+
// CHECK: "swiftmodulesDetailedInfo":[
14+
// CHECK: {
15+
// CHECK-DAG: "name":"Swift"
16+
// CHECK-DAG: "path":"{{[^"]*\\[/\\]}}Swift.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
17+
// CHECK-DAG: "isImportedDirectly":true
18+
// CHECK-DAG: "supportsLibraryEvolution":true
19+
// CHECK: }
20+
// CHECK: {
21+
// CHECK-DAG: "name":"SwiftOnoneSupport"
22+
// CHECK-DAG: "path":"{{[^"]*\\[/\\]}}SwiftOnoneSupport.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
23+
// CHECK-DAG: "isImportedDirectly":true
24+
// CHECK-DAG: "supportsLibraryEvolution":true
1225
// CHECK: }
26+
// CHECK: ]

‎test/Driver/loaded_module_trace_foundation.swift

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,32 @@
66
// CHECK: "name":"loaded_module_trace_foundation"
77
// CHECK: "arch":"{{[^"]*}}"
88
// CHECK: "swiftmodules":[
9-
// CHECK: "{{[^"]*}}/ObjectiveC.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
10-
// CHECK: "{{[^"]*}}/Dispatch.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
11-
// CHECK: "{{[^"]*}}/Darwin.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
12-
// CHECK: "{{[^"]*}}/Foundation.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
13-
// CHECK: "{{[^"]*}}/Swift.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
14-
// CHECK: "{{[^"]*}}/SwiftOnoneSupport.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
9+
// CHECK-DAG: "{{[^"]*}}/ObjectiveC.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
10+
// CHECK-DAG: "{{[^"]*}}/Dispatch.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
11+
// CHECK-DAG: "{{[^"]*}}/Darwin.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
12+
// CHECK-DAG: "{{[^"]*}}/Foundation.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
13+
// CHECK-DAG: "{{[^"]*}}/Swift.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
14+
// CHECK-DAG: "{{[^"]*}}/SwiftOnoneSupport.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
1515
// CHECK: ]
16+
// CHECK: "swiftmodulesDetailedInfo":[
17+
// CHECK: {
18+
// CHECK-DAG: "name":"Foundation"
19+
// CHECK-DAG: "path":"{{[^"]*}}/Foundation.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
20+
// CHECK-DAG: "isImportedDirectly":true
21+
// CHECK-DAG: "supportsLibraryEvolution":true
22+
// CHECK: }
23+
// CHECK: {
24+
// CHECK-DAG: "name":"Swift"
25+
// CHECK-DAG: "path":"{{[^"]*\\[/\\]}}Swift.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
26+
// CHECK-DAG: "isImportedDirectly":true
27+
// CHECK-DAG: "supportsLibraryEvolution":true
1628
// CHECK: }
29+
// CHECK: {
30+
// CHECK-DAG: "name":"SwiftOnoneSupport"
31+
// CHECK-DAG: "path":"{{[^"]*\\[/\\]}}SwiftOnoneSupport.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
32+
// CHECK-DAG: "isImportedDirectly":true
33+
// CHECK-DAG: "supportsLibraryEvolution":true
34+
// CHECK: }
35+
// CHECK: ]
1736

1837
import Foundation

‎test/Driver/loaded_module_trace_header.swift

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,24 @@
88
// CHECK: "name":"loaded_module_trace_header"
99
// CHECK: "arch":"{{[^"]*}}"
1010
// CHECK: "swiftmodules":[
11-
// CHECK: "{{[^"]*}}/ObjectiveC.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
12-
// CHECK: "{{[^"]*}}/Dispatch.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
13-
// CHECK: "{{[^"]*}}/Darwin.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
14-
// CHECK: "{{[^"]*}}/Foundation.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
15-
// CHECK: "{{[^"]*}}/Swift.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
16-
// CHECK: "{{[^"]*}}/SwiftOnoneSupport.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
11+
// CHECK-DAG: "{{[^"]*}}/ObjectiveC.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
12+
// CHECK-DAG: "{{[^"]*}}/Dispatch.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
13+
// CHECK-DAG: "{{[^"]*}}/Darwin.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
14+
// CHECK-DAG: "{{[^"]*}}/Foundation.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
15+
// CHECK-DAG: "{{[^"]*}}/Swift.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
16+
// CHECK-DAG: "{{[^"]*}}/SwiftOnoneSupport.swiftmodule{{(\\/[^"]+[.]swiftmodule)?}}"
1717
// CHECK: ]
18+
// CHECK: "swiftmodulesDetailedInfo":[
19+
// CHECK: {
20+
// CHECK-DAG: "name":"Swift"
21+
// CHECK-DAG: "path":"{{[^"]*\\[/\\]}}Swift.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
22+
// CHECK-DAG: "isImportedDirectly":true
23+
// CHECK-DAG: "supportsLibraryEvolution":true
24+
// CHECK: }
25+
// CHECK: {
26+
// CHECK-DAG: "name":"SwiftOnoneSupport"
27+
// CHECK-DAG: "path":"{{[^"]*\\[/\\]}}SwiftOnoneSupport.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
28+
// CHECK-DAG: "isImportedDirectly":true
29+
// CHECK-DAG: "supportsLibraryEvolution":true
1830
// CHECK: }
31+
// CHECK: ]

‎test/Driver/loaded_module_trace_multifile.swift

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,25 @@
55
// RUN: %FileCheck %s < %t/multifile.trace.json
66

77
// This file only imports Module2, but the other file imports Module: hopefully they both appear!
8+
// The difference between this test and the one in loaded_module_trace is that here, we test that
9+
// dependencies from multiple files in the same module are accounted for correctly. As a result,
10+
// `Module.swiftmodule` is marked as directly imported here.
811

912
// CHECK: {
13+
// CHECK: "version":2
1014
// CHECK: "name":"loaded_module_trace_multifile"
1115
// CHECK: "arch":"{{[^"]*}}"
1216
// CHECK: "swiftmodules":[
1317
// CHECK-DAG: "{{[^"]*\\[/\\]}}Module2.swiftmodule"
14-
// CHECK-DAG: "{{[^"]*\\[/\\]}}Module.swiftmodule"
1518
// CHECK-DAG: "{{[^"]*\\[/\\]}}Swift.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
1619
// CHECK-DAG: "{{[^"]*\\[/\\]}}SwiftOnoneSupport.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}"
20+
// CHECK-DAG: "{{[^"]*\\[/\\]}}Module.swiftmodule"
21+
// CHECK: ]
22+
// CHECK: "swiftmodulesDetailedInfo":[
23+
// CHECK-DAG: {"name":"Module2","path":"{{[^"]*\\[/\\]}}Module2.swiftmodule","isImportedDirectly":true,"supportsLibraryEvolution":false}
24+
// CHECK-DAG: {"name":"Swift","path":"{{[^"]*\\[/\\]}}Swift.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}","isImportedDirectly":true,"supportsLibraryEvolution":true}
25+
// CHECK-DAG: {"name":"SwiftOnoneSupport","path":"{{[^"]*\\[/\\]}}SwiftOnoneSupport.swiftmodule{{(\\[/\\][^"]+[.]swiftmodule)?}}","isImportedDirectly":true,"supportsLibraryEvolution":true}
26+
// CHECK-DAG: {"name":"Module","path":"{{[^"]*\\[/\\]}}Module.swiftmodule","isImportedDirectly":true,"supportsLibraryEvolution":false}
1727
// CHECK: ]
1828
// CHECK: }
1929

0 commit comments

Comments
 (0)