Skip to content

Commit 87d6979

Browse files
ian-twilightcoderian-twilightcoder
authored
Merge pull request #78303 from ian-twilightcoder/clang-importer-search-paths
[ClangImporter] clang's -iframework comes before builtin usr/local/include, but Swift's -Fsystem comes after
2 parents 9d43e7c + cdb42c3 commit 87d6979

30 files changed

+258
-144
lines changed

‎include/swift/AST/SearchPathOptions.h

Lines changed: 29 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -311,18 +311,16 @@ class SearchPathOptions {
311311
friend class ASTContext;
312312

313313
public:
314-
struct FrameworkSearchPath {
314+
struct SearchPath {
315315
std::string Path;
316316
bool IsSystem = false;
317-
FrameworkSearchPath(StringRef path, bool isSystem)
318-
: Path(path), IsSystem(isSystem) {}
317+
SearchPath(StringRef path, bool isSystem)
318+
: Path(path), IsSystem(isSystem) {}
319319

320-
friend bool operator ==(const FrameworkSearchPath &LHS,
321-
const FrameworkSearchPath &RHS) {
320+
friend bool operator==(const SearchPath &LHS, const SearchPath &RHS) {
322321
return LHS.Path == RHS.Path && LHS.IsSystem == RHS.IsSystem;
323322
}
324-
friend bool operator !=(const FrameworkSearchPath &LHS,
325-
const FrameworkSearchPath &RHS) {
323+
friend bool operator!=(const SearchPath &LHS, const SearchPath &RHS) {
326324
return !(LHS == RHS);
327325
}
328326
};
@@ -332,23 +330,23 @@ class SearchPathOptions {
332330

333331
/// Path to the SDK which is being built against.
334332
///
335-
/// Must me modified through setter to keep \c SearchPathLookup in sync.
333+
/// Must be modified through setter to keep \c Lookup in sync.
336334
std::string SDKPath;
337335

338336
/// Path(s) which should be searched for modules.
339337
///
340-
/// Must me modified through setter to keep \c SearchPathLookup in sync.
341-
std::vector<std::string> ImportSearchPaths;
338+
/// Must be modified through setter to keep \c Lookup in sync.
339+
std::vector<SearchPath> ImportSearchPaths;
342340

343341
/// Path(s) which should be searched for frameworks.
344342
///
345-
/// Must me modified through setter to keep \c SearchPathLookup in sync.
346-
std::vector<FrameworkSearchPath> FrameworkSearchPaths;
343+
/// Must be modified through setter to keep \c Lookup in sync.
344+
std::vector<SearchPath> FrameworkSearchPaths;
347345

348346
/// Paths to search for stdlib modules. One of these will be
349347
/// compiler-relative.
350348
///
351-
/// Must me modified through setter to keep \c SearchPathLookup in sync.
349+
/// Must be modified through setter to keep \c Lookup in sync.
352350
std::vector<std::string> RuntimeLibraryImportPaths;
353351

354352
/// When on Darwin the framework paths that are implicitly imported.
@@ -369,17 +367,16 @@ class SearchPathOptions {
369367

370368
/// Add a single import search path. Must only be called from
371369
/// \c ASTContext::addSearchPath.
372-
void addImportSearchPath(StringRef Path, llvm::vfs::FileSystem *FS) {
373-
ImportSearchPaths.push_back(Path.str());
374-
Lookup.searchPathAdded(FS, ImportSearchPaths.back(),
375-
ModuleSearchPathKind::Import, /*isSystem=*/false,
370+
void addImportSearchPath(SearchPath Path, llvm::vfs::FileSystem *FS) {
371+
ImportSearchPaths.push_back(Path);
372+
Lookup.searchPathAdded(FS, ImportSearchPaths.back().Path,
373+
ModuleSearchPathKind::Import, Path.IsSystem,
376374
ImportSearchPaths.size() - 1);
377375
}
378376

379377
/// Add a single framework search path. Must only be called from
380378
/// \c ASTContext::addSearchPath.
381-
void addFrameworkSearchPath(FrameworkSearchPath NewPath,
382-
llvm::vfs::FileSystem *FS) {
379+
void addFrameworkSearchPath(SearchPath NewPath, llvm::vfs::FileSystem *FS) {
383380
FrameworkSearchPaths.push_back(NewPath);
384381
Lookup.searchPathAdded(FS, FrameworkSearchPaths.back().Path,
385382
ModuleSearchPathKind::Framework, NewPath.IsSystem,
@@ -448,21 +445,21 @@ class SearchPathOptions {
448445
SysRoot = sysroot;
449446
}
450447

451-
ArrayRef<std::string> getImportSearchPaths() const {
448+
ArrayRef<SearchPath> getImportSearchPaths() const {
452449
return ImportSearchPaths;
453450
}
454451

455-
void setImportSearchPaths(std::vector<std::string> NewImportSearchPaths) {
452+
void setImportSearchPaths(std::vector<SearchPath> NewImportSearchPaths) {
456453
ImportSearchPaths = NewImportSearchPaths;
457454
Lookup.searchPathsDidChange();
458455
}
459456

460-
ArrayRef<FrameworkSearchPath> getFrameworkSearchPaths() const {
457+
ArrayRef<SearchPath> getFrameworkSearchPaths() const {
461458
return FrameworkSearchPaths;
462459
}
463460

464-
void setFrameworkSearchPaths(
465-
std::vector<FrameworkSearchPath> NewFrameworkSearchPaths) {
461+
void
462+
setFrameworkSearchPaths(std::vector<SearchPath> NewFrameworkSearchPaths) {
466463
FrameworkSearchPaths = NewFrameworkSearchPaths;
467464
Lookup.searchPathsDidChange();
468465
}
@@ -597,8 +594,7 @@ class SearchPathOptions {
597594
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) const;
598595

599596
private:
600-
static StringRef
601-
pathStringFromFrameworkSearchPath(const FrameworkSearchPath &next) {
597+
static StringRef pathStringFromSearchPath(const SearchPath &next) {
602598
return next.Path;
603599
};
604600

@@ -609,17 +605,16 @@ class SearchPathOptions {
609605
using llvm::hash_combine;
610606
using llvm::hash_combine_range;
611607

612-
using FrameworkPathView = ArrayRefView<FrameworkSearchPath, StringRef,
613-
pathStringFromFrameworkSearchPath>;
614-
FrameworkPathView frameworkPathsOnly{FrameworkSearchPaths};
608+
using SearchPathView =
609+
ArrayRefView<SearchPath, StringRef, pathStringFromSearchPath>;
610+
SearchPathView importPathsOnly{ImportSearchPaths};
611+
SearchPathView frameworkPathsOnly{FrameworkSearchPaths};
615612

616613
return hash_combine(SDKPath,
617-
hash_combine_range(ImportSearchPaths.begin(),
618-
ImportSearchPaths.end()),
619-
hash_combine_range(VFSOverlayFiles.begin(),
620-
VFSOverlayFiles.end()),
621-
// FIXME: Should we include the system-ness of framework
614+
// FIXME: Should we include the system-ness of
622615
// search paths too?
616+
hash_combine_range(importPathsOnly.begin(), importPathsOnly.end()),
617+
hash_combine_range(VFSOverlayFiles.begin(), VFSOverlayFiles.end()),
623618
hash_combine_range(frameworkPathsOnly.begin(),
624619
frameworkPathsOnly.end()),
625620
hash_combine_range(LibrarySearchPaths.begin(),

‎include/swift/Frontend/Frontend.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,8 @@ class CompilerInvocation {
187187
return ClangImporterOpts.ClangScannerModuleCachePath;
188188
}
189189

190-
void setImportSearchPaths(const std::vector<std::string> &Paths) {
190+
void setImportSearchPaths(
191+
const std::vector<SearchPathOptions::SearchPath> &Paths) {
191192
SearchPathOpts.setImportSearchPaths(Paths);
192193
}
193194

@@ -196,16 +197,16 @@ class CompilerInvocation {
196197
SearchPathOpts.DeserializedPathRecoverer = obfuscator;
197198
}
198199

199-
ArrayRef<std::string> getImportSearchPaths() const {
200+
ArrayRef<SearchPathOptions::SearchPath> getImportSearchPaths() const {
200201
return SearchPathOpts.getImportSearchPaths();
201202
}
202203

203204
void setFrameworkSearchPaths(
204-
const std::vector<SearchPathOptions::FrameworkSearchPath> &Paths) {
205+
const std::vector<SearchPathOptions::SearchPath> &Paths) {
205206
SearchPathOpts.setFrameworkSearchPaths(Paths);
206207
}
207208

208-
ArrayRef<SearchPathOptions::FrameworkSearchPath> getFrameworkSearchPaths() const {
209+
ArrayRef<SearchPathOptions::SearchPath> getFrameworkSearchPaths() const {
209210
return SearchPathOpts.getFrameworkSearchPaths();
210211
}
211212

‎include/swift/Option/Options.td

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,11 @@ def I : JoinedOrSeparate<["-"], "I">,
338338
def I_EQ : Joined<["-"], "I=">, Flags<[FrontendOption, ArgumentIsPath]>,
339339
Alias<I>;
340340

341+
def Isystem : Separate<["-"], "Isystem">,
342+
Flags<[FrontendOption, ArgumentIsPath, SwiftSymbolGraphExtractOption,
343+
SwiftSynthesizeInterfaceOption]>,
344+
HelpText<"Add directory to the system import search path">;
345+
341346
def import_underlying_module : Flag<["-"], "import-underlying-module">,
342347
Flags<[FrontendOption, NoInteractiveOption]>,
343348
HelpText<"Implicitly imports the Objective-C half of a module">;

‎lib/AST/ASTContext.cpp

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -824,8 +824,8 @@ ASTContext::ASTContext(
824824
#include "swift/AST/KnownIdentifiers.def"
825825

826826
// Record the initial set of search paths.
827-
for (StringRef path : SearchPathOpts.getImportSearchPaths())
828-
getImpl().SearchPathsSet[path] |= SearchPathKind::Import;
827+
for (const auto &path : SearchPathOpts.getImportSearchPaths())
828+
getImpl().SearchPathsSet[path.Path] |= SearchPathKind::Import;
829829
for (const auto &framepath : SearchPathOpts.getFrameworkSearchPaths())
830830
getImpl().SearchPathsSet[framepath.Path] |= SearchPathKind::Framework;
831831

@@ -2098,7 +2098,7 @@ void ASTContext::addSearchPath(StringRef searchPath, bool isFramework,
20982098
SearchPathOpts.addFrameworkSearchPath({searchPath, isSystem},
20992099
SourceMgr.getFileSystem().get());
21002100
} else {
2101-
SearchPathOpts.addImportSearchPath(searchPath,
2101+
SearchPathOpts.addImportSearchPath({searchPath, isSystem},
21022102
SourceMgr.getFileSystem().get());
21032103
}
21042104

@@ -2168,8 +2168,8 @@ Identifier ASTContext::getRealModuleName(Identifier key, ModuleAliasLookupOption
21682168
}
21692169

21702170
namespace {
2171-
static StringRef
2172-
pathStringFromFrameworkSearchPath(const SearchPathOptions::FrameworkSearchPath &next) {
2171+
static StringRef pathStringFromSearchPath(
2172+
const SearchPathOptions::SearchPath &next) {
21732173
return next.Path;
21742174
}
21752175
}
@@ -2183,16 +2183,16 @@ const {
21832183
llvm::StringSet<> ASTContext::getAllModuleSearchPathsSet()
21842184
const {
21852185
llvm::StringSet<> result;
2186-
auto ImportSearchPaths = SearchPathOpts.getImportSearchPaths();
2187-
result.insert(ImportSearchPaths.begin(), ImportSearchPaths.end());
2188-
2189-
// Framework paths are "special", they contain more than path strings,
2190-
// but path strings are all we care about here.
2191-
using FrameworkPathView = ArrayRefView<SearchPathOptions::FrameworkSearchPath,
2192-
StringRef,
2193-
pathStringFromFrameworkSearchPath>;
2194-
FrameworkPathView frameworkPathsOnly{
2195-
SearchPathOpts.getFrameworkSearchPaths()};
2186+
2187+
// Import and framework paths are "special", they contain more than path
2188+
// strings, but path strings are all we care about here.
2189+
using SearchPathView = ArrayRefView<SearchPathOptions::SearchPath, StringRef,
2190+
pathStringFromSearchPath>;
2191+
2192+
SearchPathView importPathsOnly{SearchPathOpts.getImportSearchPaths()};
2193+
result.insert(importPathsOnly.begin(), importPathsOnly.end());
2194+
2195+
SearchPathView frameworkPathsOnly{SearchPathOpts.getFrameworkSearchPaths()};
21962196
result.insert(frameworkPathsOnly.begin(), frameworkPathsOnly.end());
21972197

21982198
if (LangOpts.Target.isOSDarwin()) {

‎lib/AST/SearchPathOptions.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ void ModuleSearchPathLookup::rebuildLookupTable(const SearchPathOptions *Opts,
4848
clearLookupTable();
4949

5050
for (auto Entry : llvm::enumerate(Opts->getImportSearchPaths())) {
51-
addFilesInPathToLookupTable(FS, Entry.value(),
51+
addFilesInPathToLookupTable(FS, Entry.value().Path,
5252
ModuleSearchPathKind::Import,
53-
/*isSystem=*/false, Entry.index());
53+
Entry.value().IsSystem, Entry.index());
5454
}
5555

5656
for (auto Entry : llvm::enumerate(Opts->getFrameworkSearchPaths())) {
@@ -109,9 +109,11 @@ SearchPathOptions::getSDKPlatformPath(llvm::vfs::FileSystem *FS) const {
109109
}
110110

111111
void SearchPathOptions::dump(bool isDarwin) const {
112-
llvm::errs() << "Module import search paths (non system):\n";
112+
llvm::errs() << "Module import search paths:\n";
113113
for (auto Entry : llvm::enumerate(getImportSearchPaths())) {
114-
llvm::errs() << " [" << Entry.index() << "] " << Entry.value() << "\n";
114+
llvm::errs() << " [" << Entry.index() << "] "
115+
<< (Entry.value().IsSystem ? "(system) " : "(non-system) ")
116+
<< Entry.value().Path << "\n";
115117
}
116118

117119
llvm::errs() << "Framework search paths:\n";

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -206,20 +206,6 @@ namespace {
206206
return std::make_unique<HeaderParsingASTConsumer>(Impl);
207207
}
208208
bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
209-
// Prefer frameworks over plain headers.
210-
// We add search paths here instead of when building the initial invocation
211-
// so that (a) we use the same code as search paths for imported modules,
212-
// and (b) search paths are always added after -Xcc options.
213-
SearchPathOptions &searchPathOpts = Ctx.SearchPathOpts;
214-
for (const auto &framepath : searchPathOpts.getFrameworkSearchPaths()) {
215-
Importer.addSearchPath(framepath.Path, /*isFramework*/true,
216-
framepath.IsSystem);
217-
}
218-
219-
for (const auto &path : searchPathOpts.getImportSearchPaths()) {
220-
Importer.addSearchPath(path, /*isFramework*/false, /*isSystem=*/false);
221-
}
222-
223209
auto PCH =
224210
Importer.getOrCreatePCH(ImporterOpts, SwiftPCHHash, /*Cached=*/true);
225211
if (PCH.has_value()) {
@@ -890,19 +876,19 @@ importer::addCommonInvocationArguments(
890876
}
891877
}
892878

893-
if (std::optional<StringRef> R = ctx.SearchPathOpts.getWinSDKRoot()) {
879+
if (std::optional<StringRef> R = searchPathOpts.getWinSDKRoot()) {
894880
invocationArgStrs.emplace_back("-Xmicrosoft-windows-sdk-root");
895881
invocationArgStrs.emplace_back(*R);
896882
}
897-
if (std::optional<StringRef> V = ctx.SearchPathOpts.getWinSDKVersion()) {
883+
if (std::optional<StringRef> V = searchPathOpts.getWinSDKVersion()) {
898884
invocationArgStrs.emplace_back("-Xmicrosoft-windows-sdk-version");
899885
invocationArgStrs.emplace_back(*V);
900886
}
901-
if (std::optional<StringRef> R = ctx.SearchPathOpts.getVCToolsRoot()) {
887+
if (std::optional<StringRef> R = searchPathOpts.getVCToolsRoot()) {
902888
invocationArgStrs.emplace_back("-Xmicrosoft-visualc-tools-root");
903889
invocationArgStrs.emplace_back(*R);
904890
}
905-
if (std::optional<StringRef> V = ctx.SearchPathOpts.getVCToolsVersion()) {
891+
if (std::optional<StringRef> V = searchPathOpts.getVCToolsVersion()) {
906892
invocationArgStrs.emplace_back("-Xmicrosoft-visualc-tools-version");
907893
invocationArgStrs.emplace_back(*V);
908894
}
@@ -948,6 +934,24 @@ importer::addCommonInvocationArguments(
948934
for (auto extraArg : importerOpts.ExtraArgs) {
949935
invocationArgStrs.push_back(extraArg);
950936
}
937+
938+
for (const auto &framepath : searchPathOpts.getFrameworkSearchPaths()) {
939+
if (framepath.IsSystem) {
940+
invocationArgStrs.push_back("-iframework");
941+
invocationArgStrs.push_back(framepath.Path);
942+
} else {
943+
invocationArgStrs.push_back("-F" + framepath.Path);
944+
}
945+
}
946+
947+
for (const auto &path : searchPathOpts.getImportSearchPaths()) {
948+
if (path.IsSystem) {
949+
invocationArgStrs.push_back("-isystem");
950+
invocationArgStrs.push_back(path.Path);
951+
} else {
952+
invocationArgStrs.push_back("-I" + path.Path);
953+
}
954+
}
951955
}
952956

953957
bool ClangImporter::canReadPCH(StringRef PCHFilename) {

‎lib/ClangImporter/ClangModuleDependencyScanner.cpp

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,24 +61,9 @@ moduleCacheRelativeLookupModuleOutput(const ModuleID &MID, ModuleOutputKind MOK,
6161
return outputPath.str().str();
6262
}
6363

64-
// Add search paths.
65-
// Note: This is handled differently for the Clang importer itself, which
66-
// adds search paths to Clang's data structures rather than to its
67-
// command line.
68-
static void addSearchPathInvocationArguments(
64+
static void addScannerPrefixMapperInvocationArguments(
6965
std::vector<std::string> &invocationArgStrs, ASTContext &ctx) {
70-
SearchPathOptions &searchPathOpts = ctx.SearchPathOpts;
71-
for (const auto &framepath : searchPathOpts.getFrameworkSearchPaths()) {
72-
invocationArgStrs.push_back(framepath.IsSystem ? "-iframework" : "-F");
73-
invocationArgStrs.push_back(framepath.Path);
74-
}
75-
76-
for (const auto &path : searchPathOpts.getImportSearchPaths()) {
77-
invocationArgStrs.push_back("-I");
78-
invocationArgStrs.push_back(path);
79-
}
80-
81-
for (const auto &arg : searchPathOpts.ScannerPrefixMapper) {
66+
for (const auto &arg : ctx.SearchPathOpts.ScannerPrefixMapper) {
8267
std::string prefixMapArg = "-fdepscan-prefix-map=" + arg;
8368
invocationArgStrs.push_back(prefixMapArg);
8469
}
@@ -89,7 +74,7 @@ static std::vector<std::string> getClangDepScanningInvocationArguments(
8974
ASTContext &ctx, std::optional<StringRef> sourceFileName = std::nullopt) {
9075
std::vector<std::string> commandLineArgs =
9176
ClangImporter::getClangDriverArguments(ctx);
92-
addSearchPathInvocationArguments(commandLineArgs, ctx);
77+
addScannerPrefixMapperInvocationArguments(commandLineArgs, ctx);
9378

9479
auto sourceFilePos = std::find(
9580
commandLineArgs.begin(), commandLineArgs.end(),

‎lib/DriverTool/sil_func_extractor_main.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,11 @@ int sil_func_extractor_main(ArrayRef<const char *> argv, void *MainAddr) {
242242
Invocation.setMainExecutablePath(llvm::sys::fs::getMainExecutable(argv[0], MainAddr));
243243

244244
// Give the context the list of search paths to use for modules.
245-
Invocation.setImportSearchPaths(options.ImportPaths);
245+
std::vector<SearchPathOptions::SearchPath> ImportPaths;
246+
for (const auto &path : options.ImportPaths) {
247+
ImportPaths.push_back({path, /*isSystem=*/false});
248+
}
249+
Invocation.setImportSearchPaths(ImportPaths);
246250
// Set the SDK path and target if given.
247251
if (options.SDKPath.getNumOccurrences() == 0) {
248252
const char *SDKROOT = getenv("SDKROOT");

‎lib/DriverTool/sil_llvm_gen_main.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,12 @@ int sil_llvm_gen_main(ArrayRef<const char *> argv, void *MainAddr) {
129129
Invocation.setMainExecutablePath(llvm::sys::fs::getMainExecutable(argv[0], MainAddr));
130130

131131
// Give the context the list of search paths to use for modules.
132-
Invocation.setImportSearchPaths(options.ImportPaths);
133-
std::vector<SearchPathOptions::FrameworkSearchPath> FramePaths;
132+
std::vector<SearchPathOptions::SearchPath> ImportPaths;
133+
for (const auto &path : options.ImportPaths) {
134+
ImportPaths.push_back({path, /*isSystem=*/false});
135+
}
136+
Invocation.setImportSearchPaths(ImportPaths);
137+
std::vector<SearchPathOptions::SearchPath> FramePaths;
134138
for (const auto &path : options.FrameworkPaths) {
135139
FramePaths.push_back({path, /*isSystem=*/false});
136140
}

0 commit comments

Comments
 (0)