-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Interpreter] Fall back to loading Swift dylibs from /usr/lib/swift on Apple platforms. #24838
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.
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
Conversation
@swift-ci please test |
Build failed |
Build failed |
@swift-ci please test |
5e8da93
to
83570c2
Compare
@swift-ci please test |
83570c2
to
d64d138
Compare
@swift-ci please test |
Build failed |
@swift-ci please test os x platform |
Build failed |
lib/Frontend/CompilerInvocation.cpp
Outdated
SearchPathOpts.RuntimeLibraryPath = LibPath.str(); | ||
SearchPathOpts.RuntimeLibraryPaths.clear(); | ||
SearchPathOpts.RuntimeLibraryPaths.push_back(LibPath.str()); | ||
#if defined(__APPLE__) && defined(__MACH__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you do this in terms of the target triple rather than the host architecture? I know we can only JIT in-process if they're the same, but it's the principle of the thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
if (resourceDir.empty()) return false; | ||
return path.startswith(resourceDir); | ||
for (auto &RuntimeLibraryPath : ctx.SearchPathOpts.RuntimeLibraryPaths) { | ||
if (path.startswith(RuntimeLibraryPath)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should just be using RuntimeResourcePath
to begin with. The system libraries are not in the compiler resource directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right you are.
lib/Immediate/ImmediateImpl.h
Outdated
@@ -38,7 +38,7 @@ namespace immediate { | |||
/// calls or \c null if an error occurred. | |||
/// | |||
/// \param runtimeLibPath Path to search for compiler-relative stdlib dylibs. | |||
void *loadSwiftRuntime(StringRef runtimeLibPath); | |||
void *loadSwiftRuntime(const std::vector<std::string> &runtimeLibPaths); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: ArrayRef<std::string>
would be preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'll fix it.
lib/Immediate/Immediate.cpp
Outdated
void *swift::immediate::loadSwiftRuntime(StringRef runtimeLibPath) { | ||
return loadRuntimeLib("libswiftCore" LTDL_SHLIB_EXT, runtimeLibPath); | ||
static void *loadRuntimeLib(StringRef sharedLibName, | ||
const std::vector<std::string> runtimeLibPaths) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…which would have avoided the accidental copy here.
@swift-ci please test |
Build failed |
Build failed |
@jrose-apple How's that look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@swift-ci please test |
Build failed |
Build failed |
…n Apple platforms. Continue to load the dylibs next to the compiler if they exist. If they don't, then use the OS's dylibs. rdar://problem/47528005
- Check the target triple at runtime to decide whether to use the fallback. - Change isInResourceDir to actually check the resource dir. - Use ArrayRef<std::string> instead of std::vector<std::string>.
01b5703
to
9df513a
Compare
@swift-ci please test |
Build failed |
Build failed |
Forgot to mention the LLDB PR this time! |
@swift-ci please test |
Continue to load the dylibs next to the compiler if they exist. If they don't, then use the OS's dylibs.
Requires LLDB PR apple/swift-lldb#1596.
rdar://problem/47528005