-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Driver] Infer target-specific dylib names #12443
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
[Driver] Infer target-specific dylib names #12443
Conversation
When the Swift driver is invoked with the `-emit-library` option, but without an `-o` option that specifies the emitted library's filename, logic in the `getOutputFilename()` function derives a filename: `"lib" + <a plasible base name>"`, and then the value of the `LTDL_SHLIB_EXT` macro. There are two problems here: 1. Windows shared library file names, by convention, do not begin with "lib". 2. The `LTDL_SHLIB_EXT` macro is set by `llvm/cmake/modules/HandleLLVMOptions.cmake`, based on `CMAKE_SHARED_LIBRARY_SUFFIX`, a built-in CMake variable that is set at the time LLVM is configured to be built. So, if LLVM and Swift were built on a Linux machine, but the `swiftc` executable that was built was then invoked to produce a shared library for a Darwin target, the library would have a ".so" suffix, not ".dylib". (It's for this reason that the tests for this name inference, in `test/Driver/linker.swift`, must use a regular expression that matches both ".dylib" and ".so", despite specifying a Darwin `-target`.) In order to produce conventionally correct prefixes and suffixes based on the target, modify the `getOutputFilename()` function to take an `llvm::Triple` argument.
@swift-ci please smoke test |
@@ -1814,10 +1815,17 @@ static StringRef getOutputFilename(Compilation &C, | |||
BaseName = llvm::sys::path::stem(BaseInput); | |||
if (auto link = dyn_cast<LinkJobAction>(JA)) { | |||
if (link->getKind() == LinkKind::DynamicLibrary) { | |||
// FIXME: This should be target-specific. | |||
Buffer = "lib"; | |||
if (Triple.isOSWindows()) |
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.
Drive by food for thought: Why not hoist this into LLVM? How does clang do this?
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.
Oh, absolutely, I felt the same! I didn't look where the best place for this to live would be, though. I'll check what Clang does and comment back here.
Seems reasonable to me. I think Michael's right that this ought to live somewhere else in the long run, but it'd be okay to take it here for now if it looks annoying to factor out of Clang (or if Clang doesn't even do this yet). |
Sorry it took me a while to circle back to this. As far as I can tell, I don't think Clang tries nearly as hard as Swift to come up with a reasonable default output name. Instead, it simply chooses between I did find a few other spots that determine whether to use a "lib" prefix, or what file extension to use, but none have to do with determining an output file name:
So, thanks for pointing this out, it's got me wondering whether I could contribute to Clang the same sort of output file name inference as Swift uses, at which point I think it'd be a great idea to move this logic up to |
@swift-ci please smoke test and merge |
When the Swift driver is invoked with the
-emit-library
option, but without an-o
option that specifies the emitted library's filename, logic in thegetOutputFilename()
function derives a filename:"lib" + <a plasible base name>"
, and then the value of theLTDL_SHLIB_EXT
macro.There are two problems here:
LTDL_SHLIB_EXT
macro is set byllvm/cmake/modules/HandleLLVMOptions.cmake
, based onCMAKE_SHARED_LIBRARY_SUFFIX
, a built-in CMake variable that is set at the time LLVM is configured to be built. So, if LLVM and Swift were built on a Linux machine, but theswiftc
executable that was built was then invoked to produce a shared library for a Darwin target, the library would have a ".so" suffix, not ".dylib". (It's for this reason that the tests for this name inference, intest/Driver/linker.swift
, must use a regular expression that matches both ".dylib" and ".so", despite specifying a Darwin-target
.)In order to produce conventionally correct prefixes and suffixes based on the target, modify the
getOutputFilename()
function to take anllvm::Triple
argument.