Skip to content

Commit ca906d1

Browse files
akyrtziakyrtzi
committed
Add '-Fsystem' framework search option to indicate path for frameworks that should be treated as 'system'
This has the effect of propagating the search path to the clang importer as '-iframework'. It doesn't affect whether a swift module is treated as system or not, this can be done as follow-up enhancement.
1 parent ff4b055 commit ca906d1

File tree

28 files changed

+159
-57
lines changed

28 files changed

+159
-57
lines changed

‎include/swift/AST/ASTContext.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ class ASTContext {
537537
/// Adds a search path to SearchPathOpts, unless it is already present.
538538
///
539539
/// Does any proper bookkeeping to keep all module loaders up to date as well.
540-
void addSearchPath(StringRef searchPath, bool isFramework);
540+
void addSearchPath(StringRef searchPath, bool isFramework, bool isSystem);
541541

542542
/// \brief Adds a module loader to this AST context.
543543
///

‎include/swift/AST/ClangModuleLoader.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ class ClangModuleLoader : public ModuleLoader {
4444
/// -I or -F.
4545
///
4646
/// \returns true if there was an error adding the search path.
47-
virtual bool addSearchPath(StringRef newSearchPath, bool isFramework) = 0;
47+
virtual bool addSearchPath(StringRef newSearchPath, bool isFramework,
48+
bool isSystem) = 0;
4849
};
4950

5051
} // namespace swift

‎include/swift/AST/SearchPathOptions.h

+16-1
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,26 @@ class SearchPathOptions {
3030
/// \c ASTContext::addSearchPath.
3131
std::vector<std::string> ImportSearchPaths;
3232

33+
struct FrameworkSearchPath {
34+
std::string Path;
35+
bool IsSystem = false;
36+
FrameworkSearchPath(StringRef path, bool isSystem)
37+
: Path(path), IsSystem(isSystem) {}
38+
39+
friend bool operator ==(const FrameworkSearchPath &LHS,
40+
const FrameworkSearchPath &RHS) {
41+
return LHS.Path == RHS.Path && LHS.IsSystem == RHS.IsSystem;
42+
}
43+
friend bool operator !=(const FrameworkSearchPath &LHS,
44+
const FrameworkSearchPath &RHS) {
45+
return !(LHS == RHS);
46+
}
47+
};
3348
/// Path(s) which should be searched for frameworks.
3449
///
3550
/// Do not add values to this directly. Instead, use
3651
/// \c ASTContext::addSearchPath.
37-
std::vector<std::string> FrameworkSearchPaths;
52+
std::vector<FrameworkSearchPath> FrameworkSearchPaths;
3853

3954
/// Path(s) which should be searched for libraries.
4055
///

‎include/swift/ClangImporter/ClangImporter.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,8 @@ class ClangImporter final : public ClangModuleLoader {
171171
/// -I or -F.
172172
///
173173
/// \returns true if there was an error adding the search path.
174-
bool addSearchPath(StringRef newSearchPath, bool isFramework) override;
174+
bool addSearchPath(StringRef newSearchPath, bool isFramework,
175+
bool isSystem) override;
175176

176177
/// Imports an Objective-C header file into the shared imported header module.
177178
///

‎include/swift/Frontend/Frontend.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,12 @@ class CompilerInvocation {
133133
return SearchPathOpts.ImportSearchPaths;
134134
}
135135

136-
void setFrameworkSearchPaths(const std::vector<std::string> &Paths) {
136+
void setFrameworkSearchPaths(
137+
const std::vector<SearchPathOptions::FrameworkSearchPath> &Paths) {
137138
SearchPathOpts.FrameworkSearchPaths = Paths;
138139
}
139140

140-
ArrayRef<std::string> getFrameworkSearchPaths() const {
141+
ArrayRef<SearchPathOptions::FrameworkSearchPath> getFrameworkSearchPaths() const {
141142
return SearchPathOpts.FrameworkSearchPaths;
142143
}
143144

‎include/swift/Option/Options.td

+4
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,10 @@ def F : JoinedOrSeparate<["-"], "F">, Flags<[FrontendOption]>,
148148
HelpText<"Add directory to framework search path">;
149149
def F_EQ : Joined<["-"], "F=">, Flags<[FrontendOption]>, Alias<F>;
150150

151+
def Fsystem : JoinedOrSeparate<["-"], "Fsystem">, Flags<[FrontendOption]>,
152+
HelpText<"Add directory to system framework search path">;
153+
def Fsystem_EQ : Joined<["-"], "Fsystem=">, Flags<[FrontendOption]>, Alias<Fsystem>;
154+
151155
def I : JoinedOrSeparate<["-"], "I">, Flags<[FrontendOption]>,
152156
HelpText<"Add directory to the import search path">;
153157
def I_EQ : Joined<["-"], "I=">, Flags<[FrontendOption]>, Alias<I>;

‎include/swift/Serialization/ModuleFile.h

+6-1
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,16 @@ class ModuleFile : public LazyMemberLoader {
153153
/// All modules this module depends on.
154154
SmallVector<Dependency, 8> Dependencies;
155155

156+
struct SearchPath {
157+
StringRef Path;
158+
bool IsFramework;
159+
bool IsSystem;
160+
};
156161
/// Search paths this module may provide.
157162
///
158163
/// This is not intended for use by frameworks, but may show up in debug
159164
/// modules.
160-
std::vector<std::pair<StringRef, bool>> SearchPaths;
165+
std::vector<SearchPath> SearchPaths;
161166

162167
/// Info for the (lone) imported header for this module.
163168
struct {

‎include/swift/Serialization/ModuleFormat.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ const uint16_t VERSION_MAJOR = 0;
5454
/// in source control, you should also update the comment to briefly
5555
/// describe what change you made. The content of this comment isn't important;
5656
/// it just ensures a conflict if two people change the module format.
57-
const uint16_t VERSION_MINOR = 316; // Last change: Swift calling convention
57+
const uint16_t VERSION_MINOR = 317; // Last change: search path options record
58+
// if it is system or not.
5859

5960
using DeclID = PointerEmbeddedInt<unsigned, 31>;
6061
using DeclIDField = BCFixed<31>;
@@ -549,6 +550,7 @@ namespace input_block {
549550
using SearchPathLayout = BCRecordLayout<
550551
SEARCH_PATH,
551552
BCFixed<1>, // framework?
553+
BCFixed<1>, // system?
552554
BCBlob // path
553555
>;
554556
}

‎lib/AST/ASTContext.cpp

+6-5
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,8 @@ ASTContext::ASTContext(LangOptions &langOpts, SearchPathOptions &SearchPathOpts,
438438
// Record the initial set of search paths.
439439
for (StringRef path : SearchPathOpts.ImportSearchPaths)
440440
Impl.SearchPathsSet[path] |= SearchPathKind::Import;
441-
for (StringRef path : SearchPathOpts.FrameworkSearchPaths)
442-
Impl.SearchPathsSet[path] |= SearchPathKind::Framework;
441+
for (const auto &framepath : SearchPathOpts.FrameworkSearchPaths)
442+
Impl.SearchPathsSet[framepath.Path] |= SearchPathKind::Framework;
443443
}
444444

445445
ASTContext::~ASTContext() {
@@ -1145,20 +1145,21 @@ void ASTContext::setSubstitutions(TypeBase* type,
11451145
boundGenericSubstitutions[{type, gpContext}] = Subs;
11461146
}
11471147

1148-
void ASTContext::addSearchPath(StringRef searchPath, bool isFramework) {
1148+
void ASTContext::addSearchPath(StringRef searchPath, bool isFramework,
1149+
bool isSystem) {
11491150
OptionSet<SearchPathKind> &loaded = Impl.SearchPathsSet[searchPath];
11501151
auto kind = isFramework ? SearchPathKind::Framework : SearchPathKind::Import;
11511152
if (loaded.contains(kind))
11521153
return;
11531154
loaded |= kind;
11541155

11551156
if (isFramework)
1156-
SearchPathOpts.FrameworkSearchPaths.push_back(searchPath);
1157+
SearchPathOpts.FrameworkSearchPaths.push_back({searchPath, isSystem});
11571158
else
11581159
SearchPathOpts.ImportSearchPaths.push_back(searchPath);
11591160

11601161
if (auto *clangLoader = getClangModuleLoader())
1161-
clangLoader->addSearchPath(searchPath, isFramework);
1162+
clangLoader->addSearchPath(searchPath, isFramework, isSystem);
11621163
}
11631164

11641165
void ASTContext::addModuleLoader(std::unique_ptr<ModuleLoader> loader,

‎lib/ClangImporter/ClangImporter.cpp

+10-6
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,11 @@ namespace {
181181
// so that (a) we use the same code as search paths for imported modules,
182182
// and (b) search paths are always added after -Xcc options.
183183
SearchPathOptions &searchPathOpts = Ctx.SearchPathOpts;
184-
for (auto path : searchPathOpts.FrameworkSearchPaths)
185-
Importer.addSearchPath(path, /*isFramework*/true);
184+
for (const auto &framepath : searchPathOpts.FrameworkSearchPaths)
185+
Importer.addSearchPath(framepath.Path, /*isFramework*/true,
186+
framepath.IsSystem);
186187
for (auto path : searchPathOpts.ImportSearchPaths)
187-
Importer.addSearchPath(path, /*isFramework*/false);
188+
Importer.addSearchPath(path, /*isFramework*/false, /*isSystem=*/false);
188189
return true;
189190
}
190191
};
@@ -842,20 +843,23 @@ ClangImporter::create(ASTContext &ctx,
842843
return importer;
843844
}
844845

845-
bool ClangImporter::addSearchPath(StringRef newSearchPath, bool isFramework) {
846+
bool ClangImporter::addSearchPath(StringRef newSearchPath, bool isFramework,
847+
bool isSystem) {
846848
clang::FileManager &fileMgr = Impl.Instance->getFileManager();
847849
const clang::DirectoryEntry *entry = fileMgr.getDirectory(newSearchPath);
848850
if (!entry)
849851
return true;
850852

851853
auto &headerSearchInfo = Impl.getClangPreprocessor().getHeaderSearchInfo();
852-
headerSearchInfo.AddSearchPath({entry, clang::SrcMgr::C_User, isFramework},
854+
headerSearchInfo.AddSearchPath({entry, isSystem ?
855+
clang::SrcMgr::C_System :
856+
clang::SrcMgr::C_User, isFramework},
853857
/*isAngled=*/true);
854858

855859
// In addition to changing the current preprocessor directly, we still need
856860
// to change the options structure for future module-building.
857861
Impl.Instance->getHeaderSearchOpts().AddPath(newSearchPath,
858-
clang::frontend::Angled,
862+
isSystem ? clang::frontend::System : clang::frontend::Angled,
859863
isFramework,
860864
/*IgnoreSysRoot=*/true);
861865
return false;

‎lib/Driver/ToolChains.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ static void addCommonFrontendArgs(const ToolChain &TC,
124124

125125
inputArgs.AddAllArgs(arguments, options::OPT_I);
126126
inputArgs.AddAllArgs(arguments, options::OPT_F);
127+
inputArgs.AddAllArgs(arguments, options::OPT_Fsystem);
127128

128129
inputArgs.AddLastArg(arguments, options::OPT_AssertConfig);
129130
inputArgs.AddLastArg(arguments, options::OPT_autolink_force_load);
@@ -930,6 +931,8 @@ toolchains::Darwin::constructInvocation(const InterpretJobAction &job,
930931
runtimeLibraryPath);
931932
addPathEnvironmentVariableIfNeeded(II.ExtraEnvironment, "DYLD_FRAMEWORK_PATH",
932933
":", options::OPT_F, context.Args);
934+
addPathEnvironmentVariableIfNeeded(II.ExtraEnvironment, "DYLD_FRAMEWORK_PATH",
935+
":", options::OPT_Fsystem, context.Args);
933936
return II;
934937
}
935938

@@ -1119,6 +1122,7 @@ toolchains::Darwin::constructInvocation(const LinkJobAction &job,
11191122
context.Args.AddAllArgValues(Arguments, options::OPT_Xlinker);
11201123
context.Args.AddAllArgs(Arguments, options::OPT_linker_option_Group);
11211124
context.Args.AddAllArgs(Arguments, options::OPT_F);
1125+
context.Args.AddAllArgs(Arguments, options::OPT_Fsystem);
11221126

11231127
if (context.Args.hasArg(options::OPT_enable_app_extension)) {
11241128
// Keep this string fixed in case the option used by the
@@ -1441,6 +1445,7 @@ toolchains::GenericUnix::constructInvocation(const LinkJobAction &job,
14411445
context.Args.AddAllArgs(Arguments, options::OPT_Xlinker);
14421446
context.Args.AddAllArgs(Arguments, options::OPT_linker_option_Group);
14431447
context.Args.AddAllArgs(Arguments, options::OPT_F);
1448+
context.Args.AddAllArgs(Arguments, options::OPT_Fsystem);
14441449

14451450
if (!context.OI.SDKPath.empty()) {
14461451
Arguments.push_back("--sysroot");

‎lib/Frontend/CompilerInvocation.cpp

+7-1
Original file line numberDiff line numberDiff line change
@@ -1036,7 +1036,13 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts,
10361036

10371037
for (const Arg *A : make_range(Args.filtered_begin(OPT_F),
10381038
Args.filtered_end())) {
1039-
Opts.FrameworkSearchPaths.push_back(resolveSearchPath(A->getValue()));
1039+
Opts.FrameworkSearchPaths.push_back({resolveSearchPath(A->getValue()),
1040+
/*isSystem=*/false});
1041+
}
1042+
for (const Arg *A : make_range(Args.filtered_begin(OPT_Fsystem),
1043+
Args.filtered_end())) {
1044+
Opts.FrameworkSearchPaths.push_back({resolveSearchPath(A->getValue()),
1045+
/*isSystem=*/true});
10401046
}
10411047

10421048
for (const Arg *A : make_range(Args.filtered_begin(OPT_L),

‎lib/Immediate/Immediate.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ static bool tryLoadLibrary(LinkLibrary linkLib,
119119
// Try user-provided framework search paths first; frameworks contain
120120
// binaries as well as modules.
121121
for (auto &frameworkDir : searchPathOpts.FrameworkSearchPaths) {
122-
path = frameworkDir;
122+
path = frameworkDir.Path;
123123
llvm::sys::path::append(path, frameworkPart.str());
124124
success = loadRuntimeLib(path);
125125
if (success)

‎lib/Serialization/ModuleFile.cpp

+7-4
Original file line numberDiff line numberDiff line change
@@ -996,8 +996,10 @@ ModuleFile::ModuleFile(
996996
}
997997
case input_block::SEARCH_PATH: {
998998
bool isFramework;
999-
input_block::SearchPathLayout::readRecord(scratch, isFramework);
1000-
SearchPaths.push_back({blobData, isFramework});
999+
bool isSystem;
1000+
input_block::SearchPathLayout::readRecord(scratch, isFramework,
1001+
isSystem);
1002+
SearchPaths.push_back({blobData, isFramework, isSystem});
10011003
break;
10021004
}
10031005
default:
@@ -1187,8 +1189,9 @@ Status ModuleFile::associateWithFileContext(FileUnit *file,
11871189
return error(Status::TargetTooNew);
11881190
}
11891191

1190-
for (const auto &searchPathPair : SearchPaths)
1191-
ctx.addSearchPath(searchPathPair.first, searchPathPair.second);
1192+
for (const auto &searchPath : SearchPaths)
1193+
ctx.addSearchPath(searchPath.Path, searchPath.IsFramework,
1194+
searchPath.IsSystem);
11921195

11931196
auto clangImporter = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
11941197

‎lib/Serialization/Serialization.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -847,10 +847,11 @@ void Serializer::writeInputBlock(const SerializationOptions &options) {
847847
const SearchPathOptions &searchPathOpts = M->getASTContext().SearchPathOpts;
848848
// Put the framework search paths first so that they'll be preferred upon
849849
// deserialization.
850-
for (auto &path : searchPathOpts.FrameworkSearchPaths)
851-
SearchPath.emit(ScratchRecord, /*framework=*/true, path);
850+
for (auto &framepath : searchPathOpts.FrameworkSearchPaths)
851+
SearchPath.emit(ScratchRecord, /*framework=*/true, framepath.IsSystem,
852+
framepath.Path);
852853
for (auto &path : searchPathOpts.ImportSearchPaths)
853-
SearchPath.emit(ScratchRecord, /*framework=*/false, path);
854+
SearchPath.emit(ScratchRecord, /*framework=*/false, /*system=*/false, path);
854855
}
855856

856857
// FIXME: Having to deal with private imports as a superset of public imports

‎lib/Serialization/SerializedModuleLoader.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ findModule(ASTContext &ctx, AccessPathElem moduleID,
131131
moduleFramework += ".framework";
132132
isFramework = true;
133133

134-
for (auto path : ctx.SearchPathOpts.FrameworkSearchPaths) {
135-
currPath = path;
134+
for (const auto &framepath : ctx.SearchPathOpts.FrameworkSearchPaths) {
135+
currPath = framepath.Path;
136136
llvm::sys::path::append(currPath, moduleFramework.str(),
137137
"Modules", moduleFilename.str());
138138
auto err = openModuleFiles(currPath,

‎test/ClangImporter/Inputs/systemframeworks/Module.framework/Headers/Module.h

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
#ifndef MODULE_H
3+
#define MODULE_H
4+
5+
static inline int trigger_code_warning(void) {}
6+
7+
#endif // MODULE_H

‎test/ClangImporter/Inputs/systemframeworks/Module.framework/Modules/module.modulemap

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
framework module Module {
2+
umbrella header "Module.h"
3+
module * {
4+
export *
5+
}
6+
}

‎test/ClangImporter/system-framework-search-path.swift

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// The clang module triggers a warning, make sure that -Fsystem has the effect of importing as system, which will suppress the warning.
2+
3+
// RUN: rm -rf %t
4+
// RUN: mkdir -p %t
5+
6+
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -F %S/Inputs/systemframeworks -module-cache-path %t/mcp1 %s 2> %t/stderr-as-user.txt
7+
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -Fsystem %S/Inputs/systemframeworks -module-cache-path %t/mcp2 %s 2> %t/stderr-as-system.txt
8+
// RUN: %FileCheck -input-file=%t/stderr-as-user.txt %s -check-prefix=CHECK-USER
9+
// RUN: %FileCheck -input-file=%t/stderr-as-system.txt %s -check-prefix=CHECK-SYSTEM --allow-empty
10+
11+
// CHECK-USER: control reaches end of non-void function
12+
// CHECK-SYSTEM-NOT: control reaches end of non-void function
13+
import Module

‎test/Driver/driver-compile.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-macosx10.9 %s 2>&1 > %t.simple.txt
44
// RUN: %FileCheck %s < %t.simple.txt
55

6-
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-macosx10.9 %s -sdk %S/../Inputs/clang-importer-sdk -Xfrontend -foo -Xfrontend -bar -Xllvm -baz -Xcc -garply -F /path/to/frameworks -F /path/to/more/frameworks -I /path/to/headers -I path/to/more/headers -module-cache-path /tmp/modules -incremental 2>&1 > %t.complex.txt
6+
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-macosx10.9 %s -sdk %S/../Inputs/clang-importer-sdk -Xfrontend -foo -Xfrontend -bar -Xllvm -baz -Xcc -garply -F /path/to/frameworks -F /path/to/more/frameworks -Fsystem /path/to/systemframeworks -I /path/to/headers -I path/to/more/headers -module-cache-path /tmp/modules -incremental 2>&1 > %t.complex.txt
77
// RUN: %FileCheck %s < %t.complex.txt
88
// RUN: %FileCheck -check-prefix COMPLEX %s < %t.complex.txt
99

@@ -70,7 +70,7 @@
7070
// COMPLEX-DAG: -foo -bar
7171
// COMPLEX-DAG: -Xllvm -baz
7272
// COMPLEX-DAG: -Xcc -garply
73-
// COMPLEX-DAG: -F /path/to/frameworks -F /path/to/more/frameworks
73+
// COMPLEX-DAG: -F /path/to/frameworks -F /path/to/more/frameworks -Fsystem /path/to/systemframeworks
7474
// COMPLEX-DAG: -I /path/to/headers -I path/to/more/headers
7575
// COMPLEX-DAG: -module-cache-path /tmp/modules
7676
// COMPLEX-DAG: -emit-reference-dependencies-path {{(.*/)?driver-compile[^ /]+}}.swiftdeps

‎test/Serialization/search-paths-relative.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ numeric(42)
2424
// CHECK: </OPTIONS_BLOCK>
2525

2626
// CHECK-LABEL: <INPUT_BLOCK
27-
// CHECK: <SEARCH_PATH abbrevid={{[0-9]+}} op0=1/> blob data = '{{.+}}/secret/../Frameworks'
28-
// CHECK: <SEARCH_PATH abbrevid={{[0-9]+}} op0=0/> blob data = '{{.+}}/secret/.'
27+
// CHECK: <SEARCH_PATH abbrevid={{[0-9]+}} op0=1 op1=0/> blob data = '{{.+}}/secret/../Frameworks'
28+
// CHECK: <SEARCH_PATH abbrevid={{[0-9]+}} op0=0 op1=0/> blob data = '{{.+}}/secret/.'
2929
// CHECK: </INPUT_BLOCK>
3030

3131
// NEGATIVE-NOT: '.'

0 commit comments

Comments
 (0)