From cdb42c353526cb36e32dd7fba584ce53f881ac29 Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Thu, 19 Dec 2024 14:20:29 -0800 Subject: [PATCH] [ClangImporter] 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 --- include/swift/AST/SearchPathOptions.h | 63 +++++++++---------- include/swift/Frontend/Frontend.h | 9 +-- include/swift/Option/Options.td | 5 ++ lib/AST/ASTContext.cpp | 30 ++++----- lib/AST/SearchPathOptions.cpp | 10 +-- lib/ClangImporter/ClangImporter.cpp | 40 ++++++------ .../ClangModuleDependencyScanner.cpp | 21 +------ lib/DriverTool/sil_func_extractor_main.cpp | 6 +- lib/DriverTool/sil_llvm_gen_main.cpp | 8 ++- lib/DriverTool/sil_nm_main.cpp | 6 +- lib/DriverTool/sil_opt_main.cpp | 8 ++- lib/DriverTool/swift_api_digester_main.cpp | 14 ++++- .../swift_symbolgraph_extract_main.cpp | 11 +++- .../swift_synthesize_interface_main.cpp | 11 +++- lib/Frontend/CompilerInvocation.cpp | 11 ++-- lib/Sema/SourceLoader.cpp | 2 +- lib/Serialization/Serialization.cpp | 4 +- lib/Serialization/SerializedModuleLoader.cpp | 2 +- .../Module.framework/Headers/Module.h | 7 --- .../Module.framework/Modules/module.modulemap | 6 -- .../FWModule.framework/Headers/FWModule.h | 7 +++ .../Modules/module.modulemap | 4 ++ .../Inputs/systemsearchpaths/include/Module.h | 7 +++ .../include/module.modulemap | 4 ++ test/ClangImporter/search-path-order.swift | 51 +++++++++++++++ .../system-framework-search-path.swift | 12 ---- test/ClangImporter/system-search-paths.swift | 13 ++++ .../lib/SwiftLang/SwiftEditorInterfaceGen.cpp | 8 ++- tools/swift-ide-test/swift-ide-test.cpp | 16 ++++- tools/swift-refactor/swift-refactor.cpp | 6 +- 30 files changed, 258 insertions(+), 144 deletions(-) delete mode 100644 test/ClangImporter/Inputs/systemframeworks/Module.framework/Headers/Module.h delete mode 100644 test/ClangImporter/Inputs/systemframeworks/Module.framework/Modules/module.modulemap create mode 100644 test/ClangImporter/Inputs/systemsearchpaths/Frameworks/FWModule.framework/Headers/FWModule.h create mode 100644 test/ClangImporter/Inputs/systemsearchpaths/Frameworks/FWModule.framework/Modules/module.modulemap create mode 100644 test/ClangImporter/Inputs/systemsearchpaths/include/Module.h create mode 100644 test/ClangImporter/Inputs/systemsearchpaths/include/module.modulemap create mode 100644 test/ClangImporter/search-path-order.swift delete mode 100644 test/ClangImporter/system-framework-search-path.swift create mode 100644 test/ClangImporter/system-search-paths.swift diff --git a/include/swift/AST/SearchPathOptions.h b/include/swift/AST/SearchPathOptions.h index a60dbed428902..0a5d0b87a070b 100644 --- a/include/swift/AST/SearchPathOptions.h +++ b/include/swift/AST/SearchPathOptions.h @@ -311,18 +311,16 @@ class SearchPathOptions { friend class ASTContext; public: - struct FrameworkSearchPath { + struct SearchPath { std::string Path; bool IsSystem = false; - FrameworkSearchPath(StringRef path, bool isSystem) - : Path(path), IsSystem(isSystem) {} + SearchPath(StringRef path, bool isSystem) + : Path(path), IsSystem(isSystem) {} - friend bool operator ==(const FrameworkSearchPath &LHS, - const FrameworkSearchPath &RHS) { + friend bool operator==(const SearchPath &LHS, const SearchPath &RHS) { return LHS.Path == RHS.Path && LHS.IsSystem == RHS.IsSystem; } - friend bool operator !=(const FrameworkSearchPath &LHS, - const FrameworkSearchPath &RHS) { + friend bool operator!=(const SearchPath &LHS, const SearchPath &RHS) { return !(LHS == RHS); } }; @@ -332,23 +330,23 @@ class SearchPathOptions { /// Path to the SDK which is being built against. /// - /// Must me modified through setter to keep \c SearchPathLookup in sync. + /// Must be modified through setter to keep \c Lookup in sync. std::string SDKPath; /// Path(s) which should be searched for modules. /// - /// Must me modified through setter to keep \c SearchPathLookup in sync. - std::vector ImportSearchPaths; + /// Must be modified through setter to keep \c Lookup in sync. + std::vector ImportSearchPaths; /// Path(s) which should be searched for frameworks. /// - /// Must me modified through setter to keep \c SearchPathLookup in sync. - std::vector FrameworkSearchPaths; + /// Must be modified through setter to keep \c Lookup in sync. + std::vector FrameworkSearchPaths; /// Paths to search for stdlib modules. One of these will be /// compiler-relative. /// - /// Must me modified through setter to keep \c SearchPathLookup in sync. + /// Must be modified through setter to keep \c Lookup in sync. std::vector RuntimeLibraryImportPaths; /// When on Darwin the framework paths that are implicitly imported. @@ -369,17 +367,16 @@ class SearchPathOptions { /// Add a single import search path. Must only be called from /// \c ASTContext::addSearchPath. - void addImportSearchPath(StringRef Path, llvm::vfs::FileSystem *FS) { - ImportSearchPaths.push_back(Path.str()); - Lookup.searchPathAdded(FS, ImportSearchPaths.back(), - ModuleSearchPathKind::Import, /*isSystem=*/false, + void addImportSearchPath(SearchPath Path, llvm::vfs::FileSystem *FS) { + ImportSearchPaths.push_back(Path); + Lookup.searchPathAdded(FS, ImportSearchPaths.back().Path, + ModuleSearchPathKind::Import, Path.IsSystem, ImportSearchPaths.size() - 1); } /// Add a single framework search path. Must only be called from /// \c ASTContext::addSearchPath. - void addFrameworkSearchPath(FrameworkSearchPath NewPath, - llvm::vfs::FileSystem *FS) { + void addFrameworkSearchPath(SearchPath NewPath, llvm::vfs::FileSystem *FS) { FrameworkSearchPaths.push_back(NewPath); Lookup.searchPathAdded(FS, FrameworkSearchPaths.back().Path, ModuleSearchPathKind::Framework, NewPath.IsSystem, @@ -448,21 +445,21 @@ class SearchPathOptions { SysRoot = sysroot; } - ArrayRef getImportSearchPaths() const { + ArrayRef getImportSearchPaths() const { return ImportSearchPaths; } - void setImportSearchPaths(std::vector NewImportSearchPaths) { + void setImportSearchPaths(std::vector NewImportSearchPaths) { ImportSearchPaths = NewImportSearchPaths; Lookup.searchPathsDidChange(); } - ArrayRef getFrameworkSearchPaths() const { + ArrayRef getFrameworkSearchPaths() const { return FrameworkSearchPaths; } - void setFrameworkSearchPaths( - std::vector NewFrameworkSearchPaths) { + void + setFrameworkSearchPaths(std::vector NewFrameworkSearchPaths) { FrameworkSearchPaths = NewFrameworkSearchPaths; Lookup.searchPathsDidChange(); } @@ -597,8 +594,7 @@ class SearchPathOptions { llvm::IntrusiveRefCntPtr BaseFS) const; private: - static StringRef - pathStringFromFrameworkSearchPath(const FrameworkSearchPath &next) { + static StringRef pathStringFromSearchPath(const SearchPath &next) { return next.Path; }; @@ -609,17 +605,16 @@ class SearchPathOptions { using llvm::hash_combine; using llvm::hash_combine_range; - using FrameworkPathView = ArrayRefView; - FrameworkPathView frameworkPathsOnly{FrameworkSearchPaths}; + using SearchPathView = + ArrayRefView; + SearchPathView importPathsOnly{ImportSearchPaths}; + SearchPathView frameworkPathsOnly{FrameworkSearchPaths}; return hash_combine(SDKPath, - hash_combine_range(ImportSearchPaths.begin(), - ImportSearchPaths.end()), - hash_combine_range(VFSOverlayFiles.begin(), - VFSOverlayFiles.end()), - // FIXME: Should we include the system-ness of framework + // FIXME: Should we include the system-ness of // search paths too? + hash_combine_range(importPathsOnly.begin(), importPathsOnly.end()), + hash_combine_range(VFSOverlayFiles.begin(), VFSOverlayFiles.end()), hash_combine_range(frameworkPathsOnly.begin(), frameworkPathsOnly.end()), hash_combine_range(LibrarySearchPaths.begin(), diff --git a/include/swift/Frontend/Frontend.h b/include/swift/Frontend/Frontend.h index a522d841d119b..abb6d3a043002 100644 --- a/include/swift/Frontend/Frontend.h +++ b/include/swift/Frontend/Frontend.h @@ -187,7 +187,8 @@ class CompilerInvocation { return ClangImporterOpts.ClangScannerModuleCachePath; } - void setImportSearchPaths(const std::vector &Paths) { + void setImportSearchPaths( + const std::vector &Paths) { SearchPathOpts.setImportSearchPaths(Paths); } @@ -196,16 +197,16 @@ class CompilerInvocation { SearchPathOpts.DeserializedPathRecoverer = obfuscator; } - ArrayRef getImportSearchPaths() const { + ArrayRef getImportSearchPaths() const { return SearchPathOpts.getImportSearchPaths(); } void setFrameworkSearchPaths( - const std::vector &Paths) { + const std::vector &Paths) { SearchPathOpts.setFrameworkSearchPaths(Paths); } - ArrayRef getFrameworkSearchPaths() const { + ArrayRef getFrameworkSearchPaths() const { return SearchPathOpts.getFrameworkSearchPaths(); } diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td index bb77f4a5b86b1..9a5ca40a7d922 100644 --- a/include/swift/Option/Options.td +++ b/include/swift/Option/Options.td @@ -338,6 +338,11 @@ def I : JoinedOrSeparate<["-"], "I">, def I_EQ : Joined<["-"], "I=">, Flags<[FrontendOption, ArgumentIsPath]>, Alias; +def Isystem : Separate<["-"], "Isystem">, + Flags<[FrontendOption, ArgumentIsPath, SwiftSymbolGraphExtractOption, + SwiftSynthesizeInterfaceOption]>, + HelpText<"Add directory to the system import search path">; + def import_underlying_module : Flag<["-"], "import-underlying-module">, Flags<[FrontendOption, NoInteractiveOption]>, HelpText<"Implicitly imports the Objective-C half of a module">; diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 217399bb02f9c..e47b4653fb803 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -824,8 +824,8 @@ ASTContext::ASTContext( #include "swift/AST/KnownIdentifiers.def" // Record the initial set of search paths. - for (StringRef path : SearchPathOpts.getImportSearchPaths()) - getImpl().SearchPathsSet[path] |= SearchPathKind::Import; + for (const auto &path : SearchPathOpts.getImportSearchPaths()) + getImpl().SearchPathsSet[path.Path] |= SearchPathKind::Import; for (const auto &framepath : SearchPathOpts.getFrameworkSearchPaths()) getImpl().SearchPathsSet[framepath.Path] |= SearchPathKind::Framework; @@ -2098,7 +2098,7 @@ void ASTContext::addSearchPath(StringRef searchPath, bool isFramework, SearchPathOpts.addFrameworkSearchPath({searchPath, isSystem}, SourceMgr.getFileSystem().get()); } else { - SearchPathOpts.addImportSearchPath(searchPath, + SearchPathOpts.addImportSearchPath({searchPath, isSystem}, SourceMgr.getFileSystem().get()); } @@ -2168,8 +2168,8 @@ Identifier ASTContext::getRealModuleName(Identifier key, ModuleAliasLookupOption } namespace { - static StringRef - pathStringFromFrameworkSearchPath(const SearchPathOptions::FrameworkSearchPath &next) { + static StringRef pathStringFromSearchPath( + const SearchPathOptions::SearchPath &next) { return next.Path; } } @@ -2183,16 +2183,16 @@ const { llvm::StringSet<> ASTContext::getAllModuleSearchPathsSet() const { llvm::StringSet<> result; - auto ImportSearchPaths = SearchPathOpts.getImportSearchPaths(); - result.insert(ImportSearchPaths.begin(), ImportSearchPaths.end()); - - // Framework paths are "special", they contain more than path strings, - // but path strings are all we care about here. - using FrameworkPathView = ArrayRefView; - FrameworkPathView frameworkPathsOnly{ - SearchPathOpts.getFrameworkSearchPaths()}; + + // Import and framework paths are "special", they contain more than path + // strings, but path strings are all we care about here. + using SearchPathView = ArrayRefView; + + SearchPathView importPathsOnly{SearchPathOpts.getImportSearchPaths()}; + result.insert(importPathsOnly.begin(), importPathsOnly.end()); + + SearchPathView frameworkPathsOnly{SearchPathOpts.getFrameworkSearchPaths()}; result.insert(frameworkPathsOnly.begin(), frameworkPathsOnly.end()); if (LangOpts.Target.isOSDarwin()) { diff --git a/lib/AST/SearchPathOptions.cpp b/lib/AST/SearchPathOptions.cpp index ae1cc1bac5121..23bb767a5bdf2 100644 --- a/lib/AST/SearchPathOptions.cpp +++ b/lib/AST/SearchPathOptions.cpp @@ -48,9 +48,9 @@ void ModuleSearchPathLookup::rebuildLookupTable(const SearchPathOptions *Opts, clearLookupTable(); for (auto Entry : llvm::enumerate(Opts->getImportSearchPaths())) { - addFilesInPathToLookupTable(FS, Entry.value(), + addFilesInPathToLookupTable(FS, Entry.value().Path, ModuleSearchPathKind::Import, - /*isSystem=*/false, Entry.index()); + Entry.value().IsSystem, Entry.index()); } for (auto Entry : llvm::enumerate(Opts->getFrameworkSearchPaths())) { @@ -109,9 +109,11 @@ SearchPathOptions::getSDKPlatformPath(llvm::vfs::FileSystem *FS) const { } void SearchPathOptions::dump(bool isDarwin) const { - llvm::errs() << "Module import search paths (non system):\n"; + llvm::errs() << "Module import search paths:\n"; for (auto Entry : llvm::enumerate(getImportSearchPaths())) { - llvm::errs() << " [" << Entry.index() << "] " << Entry.value() << "\n"; + llvm::errs() << " [" << Entry.index() << "] " + << (Entry.value().IsSystem ? "(system) " : "(non-system) ") + << Entry.value().Path << "\n"; } llvm::errs() << "Framework search paths:\n"; diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index e9d826dd0a55f..85078e1b88fec 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -206,20 +206,6 @@ namespace { return std::make_unique(Impl); } bool BeginSourceFileAction(clang::CompilerInstance &CI) override { - // Prefer frameworks over plain headers. - // We add search paths here instead of when building the initial invocation - // so that (a) we use the same code as search paths for imported modules, - // and (b) search paths are always added after -Xcc options. - SearchPathOptions &searchPathOpts = Ctx.SearchPathOpts; - for (const auto &framepath : searchPathOpts.getFrameworkSearchPaths()) { - Importer.addSearchPath(framepath.Path, /*isFramework*/true, - framepath.IsSystem); - } - - for (const auto &path : searchPathOpts.getImportSearchPaths()) { - Importer.addSearchPath(path, /*isFramework*/false, /*isSystem=*/false); - } - auto PCH = Importer.getOrCreatePCH(ImporterOpts, SwiftPCHHash, /*Cached=*/true); if (PCH.has_value()) { @@ -890,19 +876,19 @@ importer::addCommonInvocationArguments( } } - if (std::optional R = ctx.SearchPathOpts.getWinSDKRoot()) { + if (std::optional R = searchPathOpts.getWinSDKRoot()) { invocationArgStrs.emplace_back("-Xmicrosoft-windows-sdk-root"); invocationArgStrs.emplace_back(*R); } - if (std::optional V = ctx.SearchPathOpts.getWinSDKVersion()) { + if (std::optional V = searchPathOpts.getWinSDKVersion()) { invocationArgStrs.emplace_back("-Xmicrosoft-windows-sdk-version"); invocationArgStrs.emplace_back(*V); } - if (std::optional R = ctx.SearchPathOpts.getVCToolsRoot()) { + if (std::optional R = searchPathOpts.getVCToolsRoot()) { invocationArgStrs.emplace_back("-Xmicrosoft-visualc-tools-root"); invocationArgStrs.emplace_back(*R); } - if (std::optional V = ctx.SearchPathOpts.getVCToolsVersion()) { + if (std::optional V = searchPathOpts.getVCToolsVersion()) { invocationArgStrs.emplace_back("-Xmicrosoft-visualc-tools-version"); invocationArgStrs.emplace_back(*V); } @@ -948,6 +934,24 @@ importer::addCommonInvocationArguments( for (auto extraArg : importerOpts.ExtraArgs) { invocationArgStrs.push_back(extraArg); } + + for (const auto &framepath : searchPathOpts.getFrameworkSearchPaths()) { + if (framepath.IsSystem) { + invocationArgStrs.push_back("-iframework"); + invocationArgStrs.push_back(framepath.Path); + } else { + invocationArgStrs.push_back("-F" + framepath.Path); + } + } + + for (const auto &path : searchPathOpts.getImportSearchPaths()) { + if (path.IsSystem) { + invocationArgStrs.push_back("-isystem"); + invocationArgStrs.push_back(path.Path); + } else { + invocationArgStrs.push_back("-I" + path.Path); + } + } } bool ClangImporter::canReadPCH(StringRef PCHFilename) { diff --git a/lib/ClangImporter/ClangModuleDependencyScanner.cpp b/lib/ClangImporter/ClangModuleDependencyScanner.cpp index fbda309250cfe..02cc93aa471aa 100644 --- a/lib/ClangImporter/ClangModuleDependencyScanner.cpp +++ b/lib/ClangImporter/ClangModuleDependencyScanner.cpp @@ -61,24 +61,9 @@ moduleCacheRelativeLookupModuleOutput(const ModuleID &MID, ModuleOutputKind MOK, return outputPath.str().str(); } -// Add search paths. -// Note: This is handled differently for the Clang importer itself, which -// adds search paths to Clang's data structures rather than to its -// command line. -static void addSearchPathInvocationArguments( +static void addScannerPrefixMapperInvocationArguments( std::vector &invocationArgStrs, ASTContext &ctx) { - SearchPathOptions &searchPathOpts = ctx.SearchPathOpts; - for (const auto &framepath : searchPathOpts.getFrameworkSearchPaths()) { - invocationArgStrs.push_back(framepath.IsSystem ? "-iframework" : "-F"); - invocationArgStrs.push_back(framepath.Path); - } - - for (const auto &path : searchPathOpts.getImportSearchPaths()) { - invocationArgStrs.push_back("-I"); - invocationArgStrs.push_back(path); - } - - for (const auto &arg : searchPathOpts.ScannerPrefixMapper) { + for (const auto &arg : ctx.SearchPathOpts.ScannerPrefixMapper) { std::string prefixMapArg = "-fdepscan-prefix-map=" + arg; invocationArgStrs.push_back(prefixMapArg); } @@ -89,7 +74,7 @@ static std::vector getClangDepScanningInvocationArguments( ASTContext &ctx, std::optional sourceFileName = std::nullopt) { std::vector commandLineArgs = ClangImporter::getClangDriverArguments(ctx); - addSearchPathInvocationArguments(commandLineArgs, ctx); + addScannerPrefixMapperInvocationArguments(commandLineArgs, ctx); auto sourceFilePos = std::find( commandLineArgs.begin(), commandLineArgs.end(), diff --git a/lib/DriverTool/sil_func_extractor_main.cpp b/lib/DriverTool/sil_func_extractor_main.cpp index 67d05427185d8..25eaa8402713e 100644 --- a/lib/DriverTool/sil_func_extractor_main.cpp +++ b/lib/DriverTool/sil_func_extractor_main.cpp @@ -242,7 +242,11 @@ int sil_func_extractor_main(ArrayRef argv, void *MainAddr) { Invocation.setMainExecutablePath(llvm::sys::fs::getMainExecutable(argv[0], MainAddr)); // Give the context the list of search paths to use for modules. - Invocation.setImportSearchPaths(options.ImportPaths); + std::vector ImportPaths; + for (const auto &path : options.ImportPaths) { + ImportPaths.push_back({path, /*isSystem=*/false}); + } + Invocation.setImportSearchPaths(ImportPaths); // Set the SDK path and target if given. if (options.SDKPath.getNumOccurrences() == 0) { const char *SDKROOT = getenv("SDKROOT"); diff --git a/lib/DriverTool/sil_llvm_gen_main.cpp b/lib/DriverTool/sil_llvm_gen_main.cpp index acb2e572ab444..9bc9a2debe961 100644 --- a/lib/DriverTool/sil_llvm_gen_main.cpp +++ b/lib/DriverTool/sil_llvm_gen_main.cpp @@ -129,8 +129,12 @@ int sil_llvm_gen_main(ArrayRef argv, void *MainAddr) { Invocation.setMainExecutablePath(llvm::sys::fs::getMainExecutable(argv[0], MainAddr)); // Give the context the list of search paths to use for modules. - Invocation.setImportSearchPaths(options.ImportPaths); - std::vector FramePaths; + std::vector ImportPaths; + for (const auto &path : options.ImportPaths) { + ImportPaths.push_back({path, /*isSystem=*/false}); + } + Invocation.setImportSearchPaths(ImportPaths); + std::vector FramePaths; for (const auto &path : options.FrameworkPaths) { FramePaths.push_back({path, /*isSystem=*/false}); } diff --git a/lib/DriverTool/sil_nm_main.cpp b/lib/DriverTool/sil_nm_main.cpp index 58f9d33719de0..b32a97e61a1c2 100644 --- a/lib/DriverTool/sil_nm_main.cpp +++ b/lib/DriverTool/sil_nm_main.cpp @@ -144,7 +144,11 @@ int sil_nm_main(ArrayRef argv, void *MainAddr) { Invocation.setMainExecutablePath(llvm::sys::fs::getMainExecutable(argv[0], MainAddr)); // Give the context the list of search paths to use for modules. - Invocation.setImportSearchPaths(options.ImportPaths); + std::vector ImportPaths; + for (const auto &path : options.ImportPaths) { + ImportPaths.push_back({path, /*isSystem=*/false}); + } + Invocation.setImportSearchPaths(ImportPaths); // Set the SDK path and target if given. if (options.SDKPath.getNumOccurrences() == 0) { const char *SDKROOT = getenv("SDKROOT"); diff --git a/lib/DriverTool/sil_opt_main.cpp b/lib/DriverTool/sil_opt_main.cpp index 7bec5d7a74e8d..4699eb7a59f3f 100644 --- a/lib/DriverTool/sil_opt_main.cpp +++ b/lib/DriverTool/sil_opt_main.cpp @@ -663,8 +663,12 @@ int sil_opt_main(ArrayRef argv, void *MainAddr) { llvm::sys::fs::getMainExecutable(argv[0], MainAddr)); // Give the context the list of search paths to use for modules. - Invocation.setImportSearchPaths(options.ImportPaths); - std::vector FramePaths; + std::vector ImportPaths; + for (const auto &path : options.ImportPaths) { + ImportPaths.push_back({path, /*isSystem=*/false}); + } + Invocation.setImportSearchPaths(ImportPaths); + std::vector FramePaths; for (const auto &path : options.FrameworkPaths) { FramePaths.push_back({path, /*isSystem=*/false}); } diff --git a/lib/DriverTool/swift_api_digester_main.cpp b/lib/DriverTool/swift_api_digester_main.cpp index f23fca8449217..f97512219e14a 100644 --- a/lib/DriverTool/swift_api_digester_main.cpp +++ b/lib/DriverTool/swift_api_digester_main.cpp @@ -2465,7 +2465,7 @@ class SwiftAPIDigesterInvocation { if (!ResourceDir.empty()) { InitInvoke.setRuntimeResourcePath(ResourceDir); } - std::vector FramePaths; + std::vector FramePaths; for (const auto &path : CCSystemFrameworkPaths) { FramePaths.push_back({path, /*isSystem=*/true}); } @@ -2473,12 +2473,20 @@ class SwiftAPIDigesterInvocation { for (const auto &path : BaselineFrameworkPaths) { FramePaths.push_back({path, /*isSystem=*/false}); } - InitInvoke.setImportSearchPaths(BaselineModuleInputPaths); + std::vector ImportPaths; + for (const auto &path : BaselineModuleInputPaths) { + ImportPaths.push_back({path, /*isSystem=*/false}); + } + InitInvoke.setImportSearchPaths(ImportPaths); } else { for (const auto &path : FrameworkPaths) { FramePaths.push_back({path, /*isSystem=*/false}); } - InitInvoke.setImportSearchPaths(ModuleInputPaths); + std::vector ImportPaths; + for (const auto &path : ModuleInputPaths) { + ImportPaths.push_back({path, /*isSystem=*/false}); + } + InitInvoke.setImportSearchPaths(ImportPaths); } InitInvoke.setFrameworkSearchPaths(FramePaths); if (!ModuleList.empty()) { diff --git a/lib/DriverTool/swift_symbolgraph_extract_main.cpp b/lib/DriverTool/swift_symbolgraph_extract_main.cpp index 01ea4df66f72e..1dedd9bb72202 100644 --- a/lib/DriverTool/swift_symbolgraph_extract_main.cpp +++ b/lib/DriverTool/swift_symbolgraph_extract_main.cpp @@ -119,7 +119,7 @@ int swift_symbolgraph_extract_main(ArrayRef Args, Invocation.getClangImporterOptions().ExtraArgs.push_back(A->getValue()); } - std::vector FrameworkSearchPaths; + std::vector FrameworkSearchPaths; for (const auto *A : ParsedArgs.filtered(OPT_F)) { FrameworkSearchPaths.push_back({A->getValue(), /*isSystem*/ false}); } @@ -129,7 +129,14 @@ int swift_symbolgraph_extract_main(ArrayRef Args, Invocation.setFrameworkSearchPaths(FrameworkSearchPaths); Invocation.getSearchPathOptions().LibrarySearchPaths = ParsedArgs.getAllArgValues(OPT_L); - Invocation.setImportSearchPaths(ParsedArgs.getAllArgValues(OPT_I)); + std::vector ImportSearchPaths; + for (const auto *A : ParsedArgs.filtered(OPT_I)) { + ImportSearchPaths.push_back({A->getValue(), /*isSystem*/ false}); + } + for (const auto *A : ParsedArgs.filtered(OPT_Isystem)) { + ImportSearchPaths.push_back({A->getValue(), /*isSystem*/ true}); + } + Invocation.setImportSearchPaths(ImportSearchPaths); Invocation.getLangOptions().EnableObjCInterop = Target.isOSDarwin(); Invocation.getLangOptions().DebuggerSupport = true; diff --git a/lib/DriverTool/swift_synthesize_interface_main.cpp b/lib/DriverTool/swift_synthesize_interface_main.cpp index 85b2e7022dac7..aade905c153df 100644 --- a/lib/DriverTool/swift_synthesize_interface_main.cpp +++ b/lib/DriverTool/swift_synthesize_interface_main.cpp @@ -113,7 +113,7 @@ int swift_synthesize_interface_main(ArrayRef Args, Invocation.getClangImporterOptions().ExtraArgs.push_back(A->getValue()); } - std::vector FrameworkSearchPaths; + std::vector FrameworkSearchPaths; for (const auto *A : ParsedArgs.filtered(OPT_F)) { FrameworkSearchPaths.push_back({A->getValue(), /*isSystem*/ false}); } @@ -123,7 +123,14 @@ int swift_synthesize_interface_main(ArrayRef Args, Invocation.setFrameworkSearchPaths(FrameworkSearchPaths); Invocation.getSearchPathOptions().LibrarySearchPaths = ParsedArgs.getAllArgValues(OPT_L); - Invocation.setImportSearchPaths(ParsedArgs.getAllArgValues(OPT_I)); + std::vector ImportSearchPaths; + for (const auto *A : ParsedArgs.filtered(OPT_I)) { + ImportSearchPaths.push_back({A->getValue(), /*isSystem*/ false}); + } + for (const auto *A : ParsedArgs.filtered(OPT_Isystem)) { + ImportSearchPaths.push_back({A->getValue(), /*isSystem*/ true}); + } + Invocation.setImportSearchPaths(ImportSearchPaths); Invocation.getLangOptions().EnableObjCInterop = Target.isOSDarwin(); Invocation.getLangOptions().setCxxInteropFromArgs(ParsedArgs, Diags); diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index c598c71c9af88..a8289615eb16b 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -2143,13 +2143,16 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args, return std::string(fullPath.str()); }; - std::vector ImportSearchPaths(Opts.getImportSearchPaths()); - for (const Arg *A : Args.filtered(OPT_I)) { - ImportSearchPaths.push_back(resolveSearchPath(A->getValue())); + std::vector ImportSearchPaths( + Opts.getImportSearchPaths()); + for (const Arg *A : Args.filtered(OPT_I, OPT_Isystem)) { + ImportSearchPaths.push_back( + {resolveSearchPath(A->getValue()), + /*isSystem=*/A->getOption().getID() == OPT_Isystem}); } Opts.setImportSearchPaths(ImportSearchPaths); - std::vector FrameworkSearchPaths( + std::vector FrameworkSearchPaths( Opts.getFrameworkSearchPaths()); for (const Arg *A : Args.filtered(OPT_F, OPT_Fsystem)) { FrameworkSearchPaths.push_back( diff --git a/lib/Sema/SourceLoader.cpp b/lib/Sema/SourceLoader.cpp index c528a22c12f9c..a350ed6e9e16f 100644 --- a/lib/Sema/SourceLoader.cpp +++ b/lib/Sema/SourceLoader.cpp @@ -48,7 +48,7 @@ static FileOrError findModule(ASTContext &ctx, Identifier moduleID, StringRef moduleNameRef = ctx.getRealModuleName(moduleID).str(); for (const auto &Path : ctx.SearchPathOpts.getImportSearchPaths()) { - inputFilename = Path; + inputFilename = Path.Path; llvm::sys::path::append(inputFilename, moduleNameRef); inputFilename.append(".swift"); llvm::ErrorOr> FileBufOrErr = diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index 508757e9f8ea0..0365080bc29ff 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -1330,8 +1330,8 @@ void Serializer::writeInputBlock() { SearchPath.emit(ScratchRecord, /*framework=*/true, framepath.IsSystem, PathObfuscator.obfuscate(PathMapper.remapPath(framepath.Path))); for (const auto &path : searchPathOpts.getImportSearchPaths()) - SearchPath.emit(ScratchRecord, /*framework=*/false, /*system=*/false, - PathObfuscator.obfuscate(PathMapper.remapPath(path))); + SearchPath.emit(ScratchRecord, /*framework=*/false, path.IsSystem, + PathObfuscator.obfuscate(PathMapper.remapPath(path.Path))); } // Note: We're not using StringMap here because we don't need to own the diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp index d807628d30231..077aad4e56b51 100644 --- a/lib/Serialization/SerializedModuleLoader.cpp +++ b/lib/Serialization/SerializedModuleLoader.cpp @@ -99,7 +99,7 @@ std::optional forEachModuleSearchPath( callback) { for (const auto &path : Ctx.SearchPathOpts.getImportSearchPaths()) if (auto result = - callback(path, ModuleSearchPathKind::Import, /*isSystem=*/false)) + callback(path.Path, ModuleSearchPathKind::Import, path.IsSystem)) return result; for (const auto &path : Ctx.SearchPathOpts.getFrameworkSearchPaths()) diff --git a/test/ClangImporter/Inputs/systemframeworks/Module.framework/Headers/Module.h b/test/ClangImporter/Inputs/systemframeworks/Module.framework/Headers/Module.h deleted file mode 100644 index 044e68de17250..0000000000000 --- a/test/ClangImporter/Inputs/systemframeworks/Module.framework/Headers/Module.h +++ /dev/null @@ -1,7 +0,0 @@ - -#ifndef MODULE_H -#define MODULE_H - -static inline int trigger_code_warning(void) {} - -#endif // MODULE_H diff --git a/test/ClangImporter/Inputs/systemframeworks/Module.framework/Modules/module.modulemap b/test/ClangImporter/Inputs/systemframeworks/Module.framework/Modules/module.modulemap deleted file mode 100644 index 837fccb948819..0000000000000 --- a/test/ClangImporter/Inputs/systemframeworks/Module.framework/Modules/module.modulemap +++ /dev/null @@ -1,6 +0,0 @@ -framework module Module { - umbrella header "Module.h" - module * { - export * - } -} diff --git a/test/ClangImporter/Inputs/systemsearchpaths/Frameworks/FWModule.framework/Headers/FWModule.h b/test/ClangImporter/Inputs/systemsearchpaths/Frameworks/FWModule.framework/Headers/FWModule.h new file mode 100644 index 0000000000000..a52bdba733356 --- /dev/null +++ b/test/ClangImporter/Inputs/systemsearchpaths/Frameworks/FWModule.framework/Headers/FWModule.h @@ -0,0 +1,7 @@ + +#ifndef FWMODULE_H +#define FWMODULE_H + +static inline int trigger_code_warning(void) {} // expected-user-warning{{non-void function does not return a value}} + +#endif // FWMODULE_H diff --git a/test/ClangImporter/Inputs/systemsearchpaths/Frameworks/FWModule.framework/Modules/module.modulemap b/test/ClangImporter/Inputs/systemsearchpaths/Frameworks/FWModule.framework/Modules/module.modulemap new file mode 100644 index 0000000000000..a2ef3a006f3e0 --- /dev/null +++ b/test/ClangImporter/Inputs/systemsearchpaths/Frameworks/FWModule.framework/Modules/module.modulemap @@ -0,0 +1,4 @@ +framework module FWModule { + header "FWModule.h" + export * +} diff --git a/test/ClangImporter/Inputs/systemsearchpaths/include/Module.h b/test/ClangImporter/Inputs/systemsearchpaths/include/Module.h new file mode 100644 index 0000000000000..37909f2ae2ddf --- /dev/null +++ b/test/ClangImporter/Inputs/systemsearchpaths/include/Module.h @@ -0,0 +1,7 @@ + +#ifndef MODULE_H +#define MODULE_H + +static inline int trigger_code_warning(void) {} // expected-user-warning{{non-void function does not return a value}} + +#endif // MODULE_H diff --git a/test/ClangImporter/Inputs/systemsearchpaths/include/module.modulemap b/test/ClangImporter/Inputs/systemsearchpaths/include/module.modulemap new file mode 100644 index 0000000000000..af2adb92c9175 --- /dev/null +++ b/test/ClangImporter/Inputs/systemsearchpaths/include/module.modulemap @@ -0,0 +1,4 @@ +module Module { + header "Module.h" + export * +} diff --git a/test/ClangImporter/search-path-order.swift b/test/ClangImporter/search-path-order.swift new file mode 100644 index 0000000000000..0505e485d1472 --- /dev/null +++ b/test/ClangImporter/search-path-order.swift @@ -0,0 +1,51 @@ +// Make sure that Swift's search paths order the same as the corresponding clang flags +// In particular, they should all come before the default usr/local/include + +// RUN: %empty-directory(%t) +// RUN: split-file %s %t + +// RUN: %target-clang -isysroot %t/sdk -I%t/sdk/custom/include -fmodules -fmodules-cache-path=%t/mcp1 -fsyntax-only %t/SearchPathOrder.m +// RUN: %target-clang -isysroot %t/sdk -isystem %t/sdk/custom/include -fmodules -fmodules-cache-path=%t/mcp2 -fsyntax-only %t/SearchPathOrder.m +// RUN: %target-clang -isysroot %t/sdk -F%t/sdk/custom/Frameworks -fmodules -fmodules-cache-path=%t/mcp3 -fsyntax-only %t/SearchPathOrder.m +// RUN: %target-clang -isysroot %t/sdk -iframework %t/sdk/custom/Frameworks -fmodules -fmodules-cache-path=%t/mcp4 -fsyntax-only %t/SearchPathOrder.m + +// RUN: %target-swift-frontend -sdk %t/sdk -typecheck -I %t/sdk/custom/include -module-cache-path %t/mcp5 %t/SearchPathOrder.swift +// RUN: %target-swift-frontend -sdk %t/sdk -typecheck -Isystem %t/sdk/custom/include -module-cache-path %t/mcp6 %t/SearchPathOrder.swift +// RUN: %target-swift-frontend -sdk %t/sdk -typecheck -F %t/sdk/custom/Frameworks -module-cache-path %t/mcp7 %t/SearchPathOrder.swift +// RUN: %target-swift-frontend -sdk %t/sdk -typecheck -Fsystem %t/sdk/custom/Frameworks -module-cache-path %t/mcp8 %t/SearchPathOrder.swift + +// If both clang and Swift error then it's a problem with this test. +// If only Swift errors then it's a problem with the clang importer search paths code. + +//--- sdk/usr/local/include/module.modulemap +module Module { + module bomb { header "bomb.h" } +} + +//--- sdk/usr/local/include/bomb.h +#error "bomb" + + +//--- sdk/custom/include/module.modulemap +module Module { + module good { header "good.h" } +} + +//--- sdk/custom/include/good.h + +//--- sdk/custom/Frameworks/Module.framework/Modules/module.modulemap +framework module Module { + module good { header "good.h" } +} + +//--- sdk/custom/Frameworks/Module.framework/Headers/good.h + + + + + +//--- SearchPathOrder.m +@import Module; + +//--- SearchPathOrder.swift +import Module diff --git a/test/ClangImporter/system-framework-search-path.swift b/test/ClangImporter/system-framework-search-path.swift deleted file mode 100644 index b4d7c6f750602..0000000000000 --- a/test/ClangImporter/system-framework-search-path.swift +++ /dev/null @@ -1,12 +0,0 @@ -// The clang module triggers a warning, make sure that -Fsystem has the effect of importing as system, which will suppress the warning. - -// RUN: %empty-directory(%t) - -// 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 -// 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 -// RUN: %FileCheck -input-file=%t/stderr-as-user.txt %s -check-prefix=CHECK-USER -// RUN: %FileCheck -input-file=%t/stderr-as-system.txt %s -check-prefix=CHECK-SYSTEM --allow-empty - -// CHECK-USER: non-void function does not return a value -// CHECK-SYSTEM-NOT: non-void function does not return a value -import Module diff --git a/test/ClangImporter/system-search-paths.swift b/test/ClangImporter/system-search-paths.swift new file mode 100644 index 0000000000000..c5689d62cdaf0 --- /dev/null +++ b/test/ClangImporter/system-search-paths.swift @@ -0,0 +1,13 @@ +// The clang modules trigger a warning, make sure that -Fsystem/-Isystem have the effect of importing as system, which will suppress the warning. + +// RUN: %empty-directory(%t) + +// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -F %S/Inputs/systemsearchpaths/Frameworks -I %S/Inputs/systemsearchpaths/include -module-cache-path %t/mcp1 %s 2> %t/stderr-as-user.txt +// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -Fsystem %S/Inputs/systemsearchpaths/Frameworks -Isystem %S/Inputs/systemsearchpaths/include -module-cache-path %t/mcp2 %s 2> %t/stderr-as-system.txt +// RUN: %FileCheck -input-file=%t/stderr-as-user.txt %s -check-prefix=CHECK-USER +// RUN: %FileCheck -input-file=%t/stderr-as-system.txt %s -check-prefix=CHECK-SYSTEM --allow-empty + +// CHECK-USER-COUNT-2: non-void function does not return a value +// CHECK-SYSTEM-NOT: non-void function does not return a value +import FWModule +import Module diff --git a/tools/SourceKit/lib/SwiftLang/SwiftEditorInterfaceGen.cpp b/tools/SourceKit/lib/SwiftLang/SwiftEditorInterfaceGen.cpp index 27580e1cdf319..7cfb02d6f1136 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftEditorInterfaceGen.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftEditorInterfaceGen.cpp @@ -971,8 +971,12 @@ void SwiftLangSupport::findInterfaceDocument(StringRef ModuleName, else addArgPair("-F", FramePath.Path); } - for (const auto &Path : SPOpts.getImportSearchPaths()) - addArgPair("-I", Path); + for (const auto &Path : SPOpts.getImportSearchPaths()) { + if (Path.IsSystem) + addArgPair("-Isystem", Path.Path); + else + addArgPair("-I", Path.Path); + } const auto &ClangOpts = Invocation.getClangImporterOptions(); addArgPair("-module-cache-path", ClangOpts.ModuleCachePath); diff --git a/tools/swift-ide-test/swift-ide-test.cpp b/tools/swift-ide-test/swift-ide-test.cpp index 37df87bbdab03..2ff77fb1193ab 100644 --- a/tools/swift-ide-test/swift-ide-test.cpp +++ b/tools/swift-ide-test/swift-ide-test.cpp @@ -316,6 +316,11 @@ static llvm::cl::list ImportPaths("I", llvm::cl::desc("add a directory to the import search path"), llvm::cl::cat(Category)); +static llvm::cl::list +SystemImportPaths("isystem", + llvm::cl::desc("add a directory to the system import search path"), + llvm::cl::cat(Category)); + static llvm::cl::list FrameworkPaths("F", llvm::cl::desc("add a directory to the framework search path"), @@ -4514,8 +4519,15 @@ int main(int argc, char *argv[]) { } InitInvok.getClangImporterOptions().PrecompiledHeaderOutputDir = options::PCHOutputDir; - InitInvok.setImportSearchPaths(options::ImportPaths); - std::vector FramePaths; + std::vector ImportPaths; + for (const auto &path : options::ImportPaths) { + ImportPaths.push_back({path, /*isSystem=*/false}); + } + for (const auto &path : options::SystemImportPaths) { + ImportPaths.push_back({path, /*isSystem=*/true}); + } + InitInvok.setImportSearchPaths(ImportPaths); + std::vector FramePaths; for (const auto &path : options::FrameworkPaths) { FramePaths.push_back({path, /*isSystem=*/false}); } diff --git a/tools/swift-refactor/swift-refactor.cpp b/tools/swift-refactor/swift-refactor.cpp index 1d90661753b28..c4561be0b7bf8 100644 --- a/tools/swift-refactor/swift-refactor.cpp +++ b/tools/swift-refactor/swift-refactor.cpp @@ -350,7 +350,11 @@ int main(int argc, char *argv[]) { reinterpret_cast(&anchorForGetMainExecutable))); Invocation.setSDKPath(options::SDK); - Invocation.setImportSearchPaths(options::ImportPaths); + std::vector ImportPaths; + for (const auto &path : options::ImportPaths) { + ImportPaths.push_back({path, /*isSystem=*/false}); + } + Invocation.setImportSearchPaths(ImportPaths); if (!options::Triple.empty()) Invocation.setTargetTriple(options::Triple);