Skip to content

Commit 86aba97

Browse files
ian-twilightcoderian-twilightcoder
committed
clang's -iframework comes before builtin usr/local/include, but Swift's -Fsystem comes after
When Swift passes search paths to clang, it does so directly into the HeaderSearch. That means that those paths get ordered inconsistently compared to the equivalent clang flag, and causes inconsistencies when building clang modules with clang and with Swift. Instead of touching the HeaderSearch directly, pass Swift search paths as driver flags, just do them after the -Xcc ones. Swift doesn't have a way to pass a search path to clang as -isystem, only as -I which usually isn't the right flag. Add an -Isystem Swift flag so that those paths can be passed to clang as -isystem. rdar://93951328
1 parent 419d87d commit 86aba97

21 files changed

+177
-134
lines changed

‎include/swift/AST/SearchPathOptions.h

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

313313
public:
314-
struct FrameworkSearchPath {
314+
struct SearchPath {
315315
std::string Path;
316+
bool IsFramework = false;
316317
bool IsSystem = false;
317-
FrameworkSearchPath(StringRef path, bool isSystem)
318-
: Path(path), IsSystem(isSystem) {}
318+
SearchPath(StringRef path, bool isFramework, bool isSystem)
319+
: Path(path), IsFramework(isFramework), IsSystem(isSystem) {}
319320

320-
friend bool operator ==(const FrameworkSearchPath &LHS,
321-
const FrameworkSearchPath &RHS) {
322-
return LHS.Path == RHS.Path && LHS.IsSystem == RHS.IsSystem;
321+
friend bool operator==(const SearchPath &LHS, const SearchPath &RHS) {
322+
return LHS.Path == RHS.Path && LHS.IsFramework == RHS.IsFramework &&
323+
LHS.IsSystem == RHS.IsSystem;
323324
}
324-
friend bool operator !=(const FrameworkSearchPath &LHS,
325-
const FrameworkSearchPath &RHS) {
325+
friend bool operator!=(const SearchPath &LHS, const SearchPath &RHS) {
326326
return !(LHS == RHS);
327327
}
328328
};
@@ -332,23 +332,23 @@ class SearchPathOptions {
332332

333333
/// Path to the SDK which is being built against.
334334
///
335-
/// Must me modified through setter to keep \c SearchPathLookup in sync.
335+
/// Must be modified through setter to keep \c Lookup in sync.
336336
std::string SDKPath;
337337

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

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

348348
/// Paths to search for stdlib modules. One of these will be
349349
/// compiler-relative.
350350
///
351-
/// Must me modified through setter to keep \c SearchPathLookup in sync.
351+
/// Must be modified through setter to keep \c Lookup in sync.
352352
std::vector<std::string> RuntimeLibraryImportPaths;
353353

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

370370
/// Add a single import search path. Must only be called from
371371
/// \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,
372+
void addImportSearchPath(SearchPath Path, llvm::vfs::FileSystem *FS) {
373+
ImportSearchPaths.push_back(Path);
374+
Lookup.searchPathAdded(FS, ImportSearchPaths.back().Path,
375+
ModuleSearchPathKind::Import, Path.IsSystem,
376376
ImportSearchPaths.size() - 1);
377377
}
378378

379379
/// Add a single framework search path. Must only be called from
380380
/// \c ASTContext::addSearchPath.
381-
void addFrameworkSearchPath(FrameworkSearchPath NewPath,
382-
llvm::vfs::FileSystem *FS) {
381+
void addFrameworkSearchPath(SearchPath NewPath, llvm::vfs::FileSystem *FS) {
383382
FrameworkSearchPaths.push_back(NewPath);
384383
Lookup.searchPathAdded(FS, FrameworkSearchPaths.back().Path,
385384
ModuleSearchPathKind::Framework, NewPath.IsSystem,
@@ -448,21 +447,21 @@ class SearchPathOptions {
448447
SysRoot = sysroot;
449448
}
450449

451-
ArrayRef<std::string> getImportSearchPaths() const {
450+
ArrayRef<SearchPath> getImportSearchPaths() const {
452451
return ImportSearchPaths;
453452
}
454453

455-
void setImportSearchPaths(std::vector<std::string> NewImportSearchPaths) {
454+
void setImportSearchPaths(std::vector<SearchPath> NewImportSearchPaths) {
456455
ImportSearchPaths = NewImportSearchPaths;
457456
Lookup.searchPathsDidChange();
458457
}
459458

460-
ArrayRef<FrameworkSearchPath> getFrameworkSearchPaths() const {
459+
ArrayRef<SearchPath> getFrameworkSearchPaths() const {
461460
return FrameworkSearchPaths;
462461
}
463462

464-
void setFrameworkSearchPaths(
465-
std::vector<FrameworkSearchPath> NewFrameworkSearchPaths) {
463+
void
464+
setFrameworkSearchPaths(std::vector<SearchPath> NewFrameworkSearchPaths) {
466465
FrameworkSearchPaths = NewFrameworkSearchPaths;
467466
Lookup.searchPathsDidChange();
468467
}
@@ -597,8 +596,7 @@ class SearchPathOptions {
597596
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) const;
598597

599598
private:
600-
static StringRef
601-
pathStringFromFrameworkSearchPath(const FrameworkSearchPath &next) {
599+
static StringRef pathStringFromSearchPath(const SearchPath &next) {
602600
return next.Path;
603601
};
604602

@@ -609,17 +607,16 @@ class SearchPathOptions {
609607
using llvm::hash_combine;
610608
using llvm::hash_combine_range;
611609

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

616615
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
616+
// FIXME: Should we include the system-ness of
622617
// search paths too?
618+
hash_combine_range(importPathsOnly.begin(), importPathsOnly.end()),
619+
hash_combine_range(VFSOverlayFiles.begin(), VFSOverlayFiles.end()),
623620
hash_combine_range(frameworkPathsOnly.begin(),
624621
frameworkPathsOnly.end()),
625622
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: 14 additions & 12 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

@@ -2095,10 +2095,10 @@ void ASTContext::addSearchPath(StringRef searchPath, bool isFramework,
20952095
loaded |= kind;
20962096

20972097
if (isFramework) {
2098-
SearchPathOpts.addFrameworkSearchPath({searchPath, isSystem},
2098+
SearchPathOpts.addFrameworkSearchPath({searchPath, /*isFramework=*/true, isSystem},
20992099
SourceMgr.getFileSystem().get());
21002100
} else {
2101-
SearchPathOpts.addImportSearchPath(searchPath,
2101+
SearchPathOpts.addImportSearchPath({searchPath, /*isFramework=*/false, isSystem},
21022102
SourceMgr.getFileSystem().get());
21032103
}
21042104

@@ -2169,7 +2169,7 @@ Identifier ASTContext::getRealModuleName(Identifier key, ModuleAliasLookupOption
21692169

21702170
namespace {
21712171
static StringRef
2172-
pathStringFromFrameworkSearchPath(const SearchPathOptions::FrameworkSearchPath &next) {
2172+
pathStringFromSearchPath(const SearchPathOptions::SearchPath &next) {
21732173
return next.Path;
21742174
}
21752175
}
@@ -2183,15 +2183,17 @@ const {
21832183
llvm::StringSet<> ASTContext::getAllModuleSearchPathsSet()
21842184
const {
21852185
llvm::StringSet<> result;
2186-
auto ImportSearchPaths = SearchPathOpts.getImportSearchPaths();
2187-
result.insert(ImportSearchPaths.begin(), ImportSearchPaths.end());
21882186

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,
2187+
// Import and framework paths are "special", they contain more than path strings, but path strings are all we care about here.
2188+
using SearchPathView = ArrayRefView<SearchPathOptions::SearchPath,
21922189
StringRef,
2193-
pathStringFromFrameworkSearchPath>;
2194-
FrameworkPathView frameworkPathsOnly{
2190+
pathStringFromSearchPath>;
2191+
2192+
SearchPathView importPathsOnly{
2193+
SearchPathOpts.getImportSearchPaths()};
2194+
result.insert(importPathsOnly.begin(), importPathsOnly.end());
2195+
2196+
SearchPathView frameworkPathsOnly{
21952197
SearchPathOpts.getFrameworkSearchPaths()};
21962198
result.insert(frameworkPathsOnly.begin(), frameworkPathsOnly.end());
21972199

‎lib/AST/SearchPathOptions.cpp

Lines changed: 4 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,9 @@ 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() << "] " << (Entry.value().IsSystem ? "(system) " : "(non-system) ") << Entry.value().Path << "\n";
115115
}
116116

117117
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: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -61,35 +61,11 @@ 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(
69-
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) {
82-
std::string prefixMapArg = "-fdepscan-prefix-map=" + arg;
83-
invocationArgStrs.push_back(prefixMapArg);
84-
}
85-
}
86-
8764
/// Create the command line for Clang dependency scanning.
8865
static std::vector<std::string> getClangDepScanningInvocationArguments(
8966
ASTContext &ctx, std::optional<StringRef> sourceFileName = std::nullopt) {
9067
std::vector<std::string> commandLineArgs =
9168
ClangImporter::getClangDriverArguments(ctx);
92-
addSearchPathInvocationArguments(commandLineArgs, ctx);
9369

9470
auto sourceFilePos = std::find(
9571
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, /*isFramework=*/false, /*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");

0 commit comments

Comments
 (0)