Skip to content

Fix cmake build. #352

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix cmake build. #352

wants to merge 1 commit into from

Conversation

3405691582
Copy link
Contributor

@3405691582 3405691582 commented Apr 18, 2025

Fix cmake build.

Checklist

  • I've run tests to see all new and existing tests pass
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary (not necessary)

If you've made changes to gyb files

  • I've run .script/generate_boilerplate_files_with_gyb and included updated generated files in a commit of this pull request (not necessary)

Motivation:

swift-crypto, a swiftpm dependency, is not compiling when bootstrapping swiftpm with cmake. It is however building fine when built with an existing swiftpm.

In #349 we added a new computed property to handle the fact that ManagedBuffer.capacity is unavailable on OpenBSD. In doing this, however we encountered some strange errors about this property being internal and cannot be referenced from an '@inlinable' function despite the fact we marked it @usableFromInline. Moving the computed property to the nested class extension seemed to help, but in recent versions of swift this caused a different strange compiler error about the computed property having the wrong linkage.

Modifications:

While I don't really understand what is happening here enough to say whether this is a compiler bug or not, this only seems to be triggering from _copyBytes and _withVeryUnsafeMutableBytes still. We want these to be inlinable, so the next natural workaround seems to be to plumb the capacity references in by argument instead of referencing self. This makes the callsites a bit wordy, but it seems to work.

Result:

swiftpm successfully bootstraps with cmake.

In apple#349 we added a new computed property to handle the fact that
ManagedBuffer.capacity is unavailable on OpenBSD. In doing this, however
we encountered some strange errors about this property being `internal
and cannot be referenced from an '@inlinable' function` despite the fact
we marked it `@usableFromInline`. Moving the computed property to the
nested class extension seemed to help, but in recent versions of swift
this caused a different strange compiler error about the computed
property having the `wrong linkage`.

While I don't really understand what is happening here enough to say
whether this is a compiler bug or not, this only seems to be triggering
from `_copyBytes` and `_withVeryUnsafeMutableBytes` still. We want these
to be inlinable, so the next natural workaround seems to be to plumb the
capacity references in by argument instead of referencing `self`. This
makes the callsites a bit wordy, but it seems to work.
@Lukasa
Copy link
Contributor

Lukasa commented Apr 23, 2025

Hmm, before we merge this, do you have the build output from the failing build? I'm a touch curious about what's going on here.

@3405691582
Copy link
Contributor Author

3405691582 commented Apr 23, 2025

Here's two build log outputs using the April 12 Linux nighlies installed at /tmp/toolchain and the build invocation of cmake -GNinja -DCMAKE_Swift_COMPILER=/tmp/toolchain/usr/bin/swiftc /tmp/swift-crypto/. The log1.txt output has the build output before the patch, and log2.txt output has the clean build output after the patch. Attached also is log3.txt which is the verbose output from swift build without this patch: oddly enough, this builds fine here.

Also: I think at HEAD this problem doesn't manifest as a compiler crash and instead has a "wrong linkage" error when you attempt to build, but the crash logs seem to be a bit more informative. This also might be fixed post April 12, but I haven't had a chance to update and double-check yet.

@Lukasa
Copy link
Contributor

Lukasa commented Apr 23, 2025

Thanks, I've asked one of the swift team to take a look: this feels like a Swift compiler issue to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants