Skip to content

Commit 8336c67

Browse files
redsun82redsun82
authored
Merge pull request #9762 from github/alexdenisov/vfs-followup
Swift: cleanup
2 parents 144c0d6 + 5a04d62 commit 8336c67

File tree

4 files changed

+56
-55
lines changed

4 files changed

+56
-55
lines changed

‎swift/extractor/SwiftExtractor.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ static void extractDeclarations(const SwiftExtractorConfiguration& config,
7676
auto name = getTrapFilename(module, primaryFile);
7777
llvm::StringRef filename(name);
7878
std::string tempTrapName = filename.str() + '.' + std::to_string(getpid()) + ".trap";
79-
llvm::SmallString<PATH_MAX> tempTrapPath(config.tempTrapDir);
79+
llvm::SmallString<PATH_MAX> tempTrapPath(config.getTempTrapDir());
8080
llvm::sys::path::append(tempTrapPath, tempTrapName);
8181

8282
llvm::StringRef tempTrapParent = llvm::sys::path::parent_path(tempTrapPath);
@@ -151,8 +151,7 @@ static void extractDeclarations(const SwiftExtractorConfiguration& config,
151151
}
152152
}
153153

154-
void codeql::extractSwiftFiles(const SwiftExtractorConfiguration& config,
155-
swift::CompilerInstance& compiler) {
154+
static std::unordered_set<std::string> collectInputFilenames(swift::CompilerInstance& compiler) {
156155
// The frontend can be called in many different ways.
157156
// At each invocation we only extract system and builtin modules and any input source files that
158157
// have an output associated with them.
@@ -163,7 +162,10 @@ void codeql::extractSwiftFiles(const SwiftExtractorConfiguration& config,
163162
sourceFiles.insert(input.getFileName());
164163
}
165164
}
165+
return sourceFiles;
166+
}
166167

168+
static std::unordered_set<swift::ModuleDecl*> collectModules(swift::CompilerInstance& compiler) {
167169
// getASTContext().getLoadedModules() does not provide all the modules available within the
168170
// program.
169171
// We need to iterate over all the imported modules (recursively) to see the whole "universe."
@@ -187,8 +189,15 @@ void codeql::extractSwiftFiles(const SwiftExtractorConfiguration& config,
187189
}
188190
}
189191
}
192+
return allModules;
193+
}
194+
195+
void codeql::extractSwiftFiles(const SwiftExtractorConfiguration& config,
196+
swift::CompilerInstance& compiler) {
197+
auto inputFiles = collectInputFilenames(compiler);
198+
auto modules = collectModules(compiler);
190199

191-
for (auto& module : allModules) {
200+
for (auto& module : modules) {
192201
// We only extract system and builtin modules here as the other "user" modules can be built
193202
// during the build process and then re-used at a later stage. In this case, we extract the
194203
// user code twice: once during the module build in a form of a source file, and then as
@@ -201,7 +210,7 @@ void codeql::extractSwiftFiles(const SwiftExtractorConfiguration& config,
201210
} else {
202211
for (auto file : module->getFiles()) {
203212
auto sourceFile = llvm::dyn_cast<swift::SourceFile>(file);
204-
if (!sourceFile || sourceFiles.count(sourceFile->getFilename().str()) == 0) {
213+
if (!sourceFile || inputFiles.count(sourceFile->getFilename().str()) == 0) {
205214
continue;
206215
}
207216
archiveFile(config, *sourceFile);

‎swift/extractor/SwiftExtractorConfiguration.h

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,26 +12,25 @@ struct SwiftExtractorConfiguration {
1212
// A temporary directory that exists during database creation, but is deleted once the DB is
1313
// finalized.
1414
std::string scratchDir;
15-
// A temporary directory that contains TRAP files before they are moved into their final destination.
16-
// Subdirectory of the scratchDir.
17-
std::string tempTrapDir;
15+
16+
// The original arguments passed to the extractor. Used for debugging.
17+
std::vector<std::string> frontendOptions;
18+
// The patched arguments passed to the swift::performFrontend/ Used for debugging.
19+
std::vector<std::string> patchedFrontendOptions;
20+
21+
// A temporary directory that contains TRAP files before they are moved into their final
22+
// destination.
23+
std::string getTempTrapDir() const { return scratchDir + "/swift-trap-temp"; }
1824

1925
// VFS (virtual file system) support.
2026
// A temporary directory that contains VFS files used during extraction.
21-
// Subdirectory of the scratchDir.
22-
std::string VFSDir;
27+
std::string getVFSDir() const { return scratchDir + "/swift-vfs"; }
28+
2329
// A temporary directory that contains temp VFS files before they moved into VFSDir.
24-
// Subdirectory of the scratchDir.
25-
std::string tempVFSDir;
30+
std::string getTempVFSDir() const { return scratchDir + "/swift-vfs-temp"; }
2631

2732
// A temporary directory that contains build artifacts generated by the extractor during the
2833
// overall extraction process.
29-
// Subdirectory of the scratchDir.
30-
std::string tempArtifactDir;
31-
32-
// The original arguments passed to the extractor. Used for debugging.
33-
std::vector<std::string> frontendOptions;
34-
// The patched arguments passed to the swift::performFrontend/ Used for debugging.
35-
std::vector<std::string> patchedFrontendOptions;
34+
std::string getTempArtifactDir() const { return scratchDir + "/swift-extraction-artifacts"; }
3635
};
3736
} // namespace codeql

‎swift/extractor/SwiftOutputRewrite.cpp

Lines changed: 29 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,12 @@ static std::optional<std::string> rewriteOutputFileMap(
1919
const std::string& outputFileMapPath,
2020
const std::vector<std::string>& inputs,
2121
std::unordered_map<std::string, std::string>& remapping) {
22-
auto newPath = config.tempArtifactDir + '/' + outputFileMapPath;
22+
auto newMapPath = config.getTempArtifactDir() + '/' + outputFileMapPath;
2323

2424
// TODO: do not assume absolute path for the second parameter
2525
auto outputMapOrError = swift::OutputFileMap::loadFromPath(outputFileMapPath, "");
2626
if (!outputMapOrError) {
27+
std::cerr << "Cannot load output map: '" << outputFileMapPath << "'\n";
2728
return std::nullopt;
2829
}
2930
auto oldOutputMap = outputMapOrError.get();
@@ -39,23 +40,23 @@ static std::optional<std::string> rewriteOutputFileMap(
3940
newMap.copyFrom(*oldMap);
4041
for (auto& entry : newMap) {
4142
auto oldPath = entry.getSecond();
42-
auto newPath = config.tempArtifactDir + '/' + oldPath;
43+
auto newPath = config.getTempArtifactDir() + '/' + oldPath;
4344
entry.getSecond() = newPath;
4445
remapping[oldPath] = newPath;
4546
}
4647
}
4748
std::error_code ec;
48-
llvm::SmallString<PATH_MAX> filepath(newPath);
49+
llvm::SmallString<PATH_MAX> filepath(newMapPath);
4950
llvm::StringRef parent = llvm::sys::path::parent_path(filepath);
5051
if (std::error_code ec = llvm::sys::fs::create_directories(parent)) {
5152
std::cerr << "Cannot create relocated output map dir: '" << parent.str()
5253
<< "': " << ec.message() << "\n";
5354
return std::nullopt;
5455
}
5556

56-
llvm::raw_fd_ostream fd(newPath, ec, llvm::sys::fs::OF_None);
57+
llvm::raw_fd_ostream fd(newMapPath, ec, llvm::sys::fs::OF_None);
5758
newOutputMap.write(fd, keys);
58-
return newPath;
59+
return newMapPath;
5960
}
6061

6162
// This is an Xcode-specific workaround to produce alias names for an existing .swiftmodule file.
@@ -132,30 +133,27 @@ static std::vector<std::string> computeModuleAliases(llvm::StringRef modulePath,
132133
relocatedModulePath += "/Products/";
133134
relocatedModulePath += destinationDir + '/';
134135

135-
std::vector<std::string> moduleLocations;
136-
137-
std::string firstCase = relocatedModulePath;
138-
firstCase += moduleNameWithExt.str() + '/';
139-
moduleLocations.push_back(firstCase);
140-
141-
std::string secondCase = relocatedModulePath;
142-
secondCase += moduleName.str() + '/';
143-
secondCase += moduleNameWithExt.str() + '/';
144-
moduleLocations.push_back(secondCase);
136+
// clang-format off
137+
std::vector<std::string> moduleLocations = {
138+
// First case
139+
relocatedModulePath + moduleNameWithExt.str() + '/',
140+
// Second case
141+
relocatedModulePath + moduleName.str() + '/' + moduleNameWithExt.str() + '/',
142+
// Third case
143+
relocatedModulePath + moduleName.str() + '/' + moduleName.str() + ".framework/Modules/" + moduleNameWithExt.str() + '/',
144+
};
145+
// clang-format on
145146

146-
std::string thirdCase = relocatedModulePath;
147-
thirdCase += moduleName.str() + '/';
148-
thirdCase += moduleName.str() + ".framework/Modules/";
149-
thirdCase += moduleNameWithExt.str() + '/';
150-
moduleLocations.push_back(thirdCase);
147+
std::vector<std::string> archs({arch});
148+
if (!targetTriple.empty()) {
149+
llvm::Triple triple(targetTriple);
150+
archs.push_back(swift::getTargetSpecificModuleTriple(triple).normalize());
151+
}
151152

152153
std::vector<std::string> aliases;
153154
for (auto& location : moduleLocations) {
154-
aliases.push_back(location + arch + ".swiftmodule");
155-
if (!targetTriple.empty()) {
156-
llvm::Triple triple(targetTriple);
157-
auto moduleTriple = swift::getTargetSpecificModuleTriple(triple);
158-
aliases.push_back(location + moduleTriple.normalize() + ".swiftmodule");
155+
for (auto& a : archs) {
156+
aliases.push_back(location + a + ".swiftmodule");
159157
}
160158
}
161159

@@ -195,7 +193,7 @@ std::unordered_map<std::string, std::string> rewriteOutputsInPlace(
195193
for (size_t i = 0; i < CLIArgs.size(); i++) {
196194
if (pathRewriteOptions.count(CLIArgs[i])) {
197195
auto oldPath = CLIArgs[i + 1];
198-
auto newPath = config.tempArtifactDir + '/' + oldPath;
196+
auto newPath = config.getTempArtifactDir() + '/' + oldPath;
199197
CLIArgs[++i] = newPath;
200198
newLocations.push_back(newPath);
201199

@@ -261,20 +259,20 @@ void storeRemappingForVFS(const SwiftExtractorConfiguration& config,
261259
return;
262260
}
263261

264-
if (std::error_code ec = llvm::sys::fs::create_directories(config.tempVFSDir)) {
262+
if (std::error_code ec = llvm::sys::fs::create_directories(config.getTempVFSDir())) {
265263
std::cerr << "Cannot create temp VFS directory: " << ec.message() << "\n";
266264
return;
267265
}
268266

269-
if (std::error_code ec = llvm::sys::fs::create_directories(config.VFSDir)) {
267+
if (std::error_code ec = llvm::sys::fs::create_directories(config.getVFSDir())) {
270268
std::cerr << "Cannot create VFS directory: " << ec.message() << "\n";
271269
return;
272270
}
273271

274272
// Constructing the VFS yaml file in a temp folder so that the other process doesn't read it
275273
// while it is not complete
276274
// TODO: Pick a more robust way to not collide with files from other processes
277-
auto tempVfsPath = config.tempVFSDir + '/' + std::to_string(getpid()) + "-vfs.yaml";
275+
auto tempVfsPath = config.getTempVFSDir() + '/' + std::to_string(getpid()) + "-vfs.yaml";
278276
std::error_code ec;
279277
llvm::raw_fd_ostream fd(tempVfsPath, ec, llvm::sys::fs::OF_None);
280278
if (ec) {
@@ -299,7 +297,7 @@ void storeRemappingForVFS(const SwiftExtractorConfiguration& config,
299297
fd << "}\n";
300298

301299
fd.flush();
302-
auto vfsPath = config.VFSDir + '/' + std::to_string(getpid()) + "-vfs.yaml";
300+
auto vfsPath = config.getVFSDir() + '/' + std::to_string(getpid()) + "-vfs.yaml";
303301
if (std::error_code ec = llvm::sys::fs::rename(tempVfsPath, vfsPath)) {
304302
std::cerr << "Cannot move temp VFS file '" << tempVfsPath << "' -> '" << vfsPath
305303
<< "': " << ec.message() << "\n";
@@ -308,7 +306,7 @@ void storeRemappingForVFS(const SwiftExtractorConfiguration& config,
308306
}
309307

310308
std::vector<std::string> collectVFSFiles(const SwiftExtractorConfiguration& config) {
311-
auto vfsDir = config.VFSDir + '/';
309+
auto vfsDir = config.getVFSDir() + '/';
312310
if (!llvm::sys::fs::exists(vfsDir)) {
313311
return {};
314312
}

‎swift/extractor/main.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,6 @@ int main(int argc, char** argv) {
5858
configuration.sourceArchiveDir = getenv_or("CODEQL_EXTRACTOR_SWIFT_SOURCE_ARCHIVE_DIR", ".");
5959
configuration.scratchDir = getenv_or("CODEQL_EXTRACTOR_SWIFT_SCRATCH_DIR", ".");
6060

61-
configuration.tempTrapDir = configuration.scratchDir + "/swift-trap-temp";
62-
configuration.VFSDir = configuration.scratchDir + "/swift-vfs";
63-
configuration.tempVFSDir = configuration.scratchDir + "/swift-vfs-temp";
64-
configuration.tempArtifactDir = configuration.scratchDir + "/swift-extraction-artifacts";
65-
6661
configuration.frontendOptions.reserve(argc - 1);
6762
for (int i = 1; i < argc; i++) {
6863
configuration.frontendOptions.push_back(argv[i]);

0 commit comments

Comments
 (0)