Skip to content

Have the frontend and new swift-driver look in an external -sdk for non-Darwin platform runtime libraries and modules too #79621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
+18 −13
Draft

Have the frontend and new swift-driver look in an external -sdk for non-Darwin platform runtime libraries and modules too #79621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbf58b9
Have the frontend and new swift-driver look in an external `-sdk` for…
finagolfin Feb 25, 2025
32f01c9
Don't check for the OS in the Swift resource directory and fix apinot…
finagolfin Mar 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions lib/ClangImporter/ClangImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,23 +753,11 @@ void importer::getNormalInvocationArguments(
invocationArgStrs.push_back("-fapinotes-swift-version=" +
languageVersion.asAPINotesVersionString());

// Prefer `-sdk` paths.
if (!searchPathOpts.getSDKPath().empty()) {
llvm::SmallString<261> path{searchPathOpts.getSDKPath()};
llvm::sys::path::append(path, "usr", "lib", "swift", "apinotes");

invocationArgStrs.push_back("-iapinotes-modules");
invocationArgStrs.push_back(path.str().str());
}

// Fallback to "legacy" `-resource-dir` paths.
{
llvm::SmallString<261> path{searchPathOpts.RuntimeResourcePath};
llvm::sys::path::append(path, "apinotes");

invocationArgStrs.push_back("-iapinotes-modules");
invocationArgStrs.push_back(path.str().str());
}
}

static void
Expand Down
19 changes: 18 additions & 1 deletion lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2166,6 +2166,7 @@ static bool validateSwiftModuleFileArgumentAndAdd(const std::string &swiftModule

static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
DiagnosticEngine &Diags,
const llvm::Triple &Triple,
const CASOptions &CASOpts,
const FrontendOptions &FrontendOpts,
StringRef workingDirectory) {
Expand Down Expand Up @@ -2300,6 +2301,22 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,

if (const Arg *A = Args.getLastArg(OPT_resource_dir))
Opts.RuntimeResourcePath = A->getValue();
else if (!Triple.isOSDarwin() && Args.hasArg(OPT_sdk)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code explicitly follows what the original C++ Driver has long done when looking for the Swift runtime libraries, swiftrt.o, and a few other files found in the Swift resource directory:

if (const Arg *A = args.getLastArg(options::OPT_resource_dir)) {
    StringRef value = A->getValue();
    resourceDirPath.append(value.begin(), value.end());
  } else if (!getTriple().isOSDarwin() && args.hasArg(options::OPT_sdk)) {
    StringRef value = args.getLastArg(options::OPT_sdk)->getValue();
    resourceDirPath.append(value.begin(), value.end());
    llvm::sys::path::append(resourceDirPath, "usr");
    CompilerInvocation::appendSwiftLibDir(resourceDirPath, shared);
  } else {
    auto programPath = getDriver().getSwiftProgramPath();
    CompilerInvocation::computeRuntimeResourcePathFromExecutablePath(
        programPath, shared, resourceDirPath);
  }

Note how the SDK is only looked in if a non-Darwin -sdk is explicitly specified: Saleem later tried to expand that to Darwin also in #26361, but he may have never got it to work.

That C++ Driver setup now matches this Frontend setup, because the default in both is now to look relative to the compiler, which is done first in this Frontend here, ie usr/bin/../lib/swift/. If a -resource-dir is set, that is given first priority, then a non-Darwin -sdk is given second priority, ie the C++ Driver and the Frontend now match in where they look.

This is important for two reasons:

  1. The new swift-driver simply queries the Frontend and uses whatever Swift resource directory it uses, so now the new swift-driver finally matches the original C++ Driver's behavior, and piecemeal workarounds like that in [Unix] Go back to only checking the runtime resource path for swiftrt.o swift-driver#1822 can now be eliminated.
  2. The Frontend will now look in the same Swift resource directory for stdlib/corelibs swiftmodules as the swift-driver is looking for runtime libraries and swiftrt.o, eliminating any confusion between the two by centralizing the Swift resource directory lookup here. That already found one bug in the Windows CI, see my other code comment.

However, unlike the C++ Driver, my -sdk code below actually checks if the -sdk path contains a Swift resource directory for the platform triple and does not use the -sdk for this if not, falling back to the aforementioned default next to the compiler in that case. This is because an -sdk is not guaranteed to have a Swift resource directory and may have only a C/C++ sysroot.

We should probably tighten this up to require an explicit -sdk to have a Swift resource directory, with the only exception when an explicit -resource-dir is also specified, but I'm open to debate here. The C++ Driver doesn't even check if the non-Darwin -sdk has a Swift resource directory and simply assumes one is there, we can do a bit better than that.

llvm::SmallString<128> SDKResourcePath(Opts.getSDKPath());
llvm::sys::path::append(
SDKResourcePath, "usr", "lib",
FrontendOpts.UseSharedResourceFolder ? "swift" : "swift_static");
// Check for eg <sdkRoot>/usr/lib/swift/
if (llvm::sys::fs::exists(SDKResourcePath))
Opts.RuntimeResourcePath = SDKResourcePath.str();
else {
llvm::outs() << "You passed in an external -sdk without a Swift runtime.\n";
llvm::outs() << "Either specify a directory containing the runtime libraries with\n";
llvm::outs() << "the -resource-dir flag, or use -sysroot instead to point at a C/C++\n";
llvm::outs() << "sysroot alone. Falling back to << Opts.RuntimeResourcePath << for\n";
llvm::outs() << "the Swift runtime modules and libraries.\n";
}
}

Opts.SkipRuntimeLibraryImportPaths |= Args.hasArg(OPT_nostdimport);
Opts.ExcludeSDKPathsFromRuntimeLibraryImportPaths |= Args.hasArg(OPT_nostdlibimport);
Expand Down Expand Up @@ -3908,7 +3925,7 @@ bool CompilerInvocation::parseArgs(

ParseSymbolGraphArgs(SymbolGraphOpts, ParsedArgs, Diags, LangOpts);

if (ParseSearchPathArgs(SearchPathOpts, ParsedArgs, Diags,
if (ParseSearchPathArgs(SearchPathOpts, ParsedArgs, Diags, LangOpts.Target,
CASOpts, FrontendOpts, workingDirectory)) {
return true;
}
Expand Down