Skip to content

Commit 5b5c247

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 5b5c247

21 files changed

+181
-134
lines changed

‎include/swift/AST/SearchPathOptions.h

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -311,44 +311,48 @@ 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
}
328+
329+
operator std::string() const {
330+
return Path;
331+
}
328332
};
329333

330334
private:
331335
ModuleSearchPathLookup Lookup;
332336

333337
/// Path to the SDK which is being built against.
334338
///
335-
/// Must me modified through setter to keep \c SearchPathLookup in sync.
339+
/// Must be modified through setter to keep \c Lookup in sync.
336340
std::string SDKPath;
337341

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

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

348352
/// Paths to search for stdlib modules. One of these will be
349353
/// compiler-relative.
350354
///
351-
/// Must me modified through setter to keep \c SearchPathLookup in sync.
355+
/// Must be modified through setter to keep \c Lookup in sync.
352356
std::vector<std::string> RuntimeLibraryImportPaths;
353357

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

370374
/// Add a single import search path. Must only be called from
371375
/// \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,
376+
void addImportSearchPath(SearchPath Path, llvm::vfs::FileSystem *FS) {
377+
ImportSearchPaths.push_back(Path);
378+
Lookup.searchPathAdded(FS, ImportSearchPaths.back().Path,
379+
ModuleSearchPathKind::Import, Path.IsSystem,
376380
ImportSearchPaths.size() - 1);
377381
}
378382

379383
/// Add a single framework search path. Must only be called from
380384
/// \c ASTContext::addSearchPath.
381-
void addFrameworkSearchPath(FrameworkSearchPath NewPath,
382-
llvm::vfs::FileSystem *FS) {
385+
void addFrameworkSearchPath(SearchPath NewPath, llvm::vfs::FileSystem *FS) {
383386
FrameworkSearchPaths.push_back(NewPath);
384387
Lookup.searchPathAdded(FS, FrameworkSearchPaths.back().Path,
385388
ModuleSearchPathKind::Framework, NewPath.IsSystem,
@@ -448,21 +451,21 @@ class SearchPathOptions {
448451
SysRoot = sysroot;
449452
}
450453

451-
ArrayRef<std::string> getImportSearchPaths() const {
454+
ArrayRef<SearchPath> getImportSearchPaths() const {
452455
return ImportSearchPaths;
453456
}
454457

455-
void setImportSearchPaths(std::vector<std::string> NewImportSearchPaths) {
458+
void setImportSearchPaths(std::vector<SearchPath> NewImportSearchPaths) {
456459
ImportSearchPaths = NewImportSearchPaths;
457460
Lookup.searchPathsDidChange();
458461
}
459462

460-
ArrayRef<FrameworkSearchPath> getFrameworkSearchPaths() const {
463+
ArrayRef<SearchPath> getFrameworkSearchPaths() const {
461464
return FrameworkSearchPaths;
462465
}
463466

464-
void setFrameworkSearchPaths(
465-
std::vector<FrameworkSearchPath> NewFrameworkSearchPaths) {
467+
void
468+
setFrameworkSearchPaths(std::vector<SearchPath> NewFrameworkSearchPaths) {
466469
FrameworkSearchPaths = NewFrameworkSearchPaths;
467470
Lookup.searchPathsDidChange();
468471
}
@@ -597,8 +600,7 @@ class SearchPathOptions {
597600
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) const;
598601

599602
private:
600-
static StringRef
601-
pathStringFromFrameworkSearchPath(const FrameworkSearchPath &next) {
603+
static StringRef pathStringFromSearchPath(const SearchPath &next) {
602604
return next.Path;
603605
};
604606

@@ -609,17 +611,16 @@ class SearchPathOptions {
609611
using llvm::hash_combine;
610612
using llvm::hash_combine_range;
611613

612-
using FrameworkPathView = ArrayRefView<FrameworkSearchPath, StringRef,
613-
pathStringFromFrameworkSearchPath>;
614-
FrameworkPathView frameworkPathsOnly{FrameworkSearchPaths};
614+
using SearchPathView =
615+
ArrayRefView<SearchPath, StringRef, pathStringFromSearchPath>;
616+
SearchPathView importPathsOnly{ImportSearchPaths};
617+
SearchPathView frameworkPathsOnly{FrameworkSearchPaths};
615618

616619
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
620+
// FIXME: Should we include the system-ness of
622621
// search paths too?
622+
hash_combine_range(importPathsOnly.begin(), importPathsOnly.end()),
623+
hash_combine_range(VFSOverlayFiles.begin(), VFSOverlayFiles.end()),
623624
hash_combine_range(frameworkPathsOnly.begin(),
624625
frameworkPathsOnly.end()),
625626
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)