Skip to content

TDBGen: add a workaround for a workaround for async PWT #64920

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

Merged
merged 1 commit into from
Apr 7, 2023

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Apr 4, 2023

When a protocol witness thunk is formed to a serialised protocol containing an async function, the async function pointer to the conformance needs to be made public due to a SIL Verifier check failure (reference to a non-fragile function from within a public fragile function). Add a stop-gap solution of rolling an extra emission for a private symbol as a public symbol to avoid the error (the underlying issue has been open for ~6y and counting as of this commit).

This was identified by swift-package-manager (#64900).

@compnerd compnerd force-pushed the public-your-private branch from 3793566 to ca7850b Compare April 4, 2023 20:30
@compnerd compnerd marked this pull request as ready for review April 4, 2023 20:30
@compnerd
Copy link
Member Author

compnerd commented Apr 4, 2023

@compnerd
Copy link
Member Author

compnerd commented Apr 4, 2023

@swift-ci please test

@compnerd compnerd force-pushed the public-your-private branch from ca7850b to f50ed93 Compare April 4, 2023 20:45
@compnerd
Copy link
Member Author

compnerd commented Apr 4, 2023

@swift-ci please test

@compnerd
Copy link
Member Author

compnerd commented Apr 4, 2023

@swift-ci please test Windows platform

@compnerd compnerd force-pushed the public-your-private branch from f50ed93 to 2b90205 Compare April 4, 2023 22:15
@compnerd
Copy link
Member Author

compnerd commented Apr 4, 2023

@swift-ci please test

@DougGregor
Copy link
Member

Will we need this on release/5.9? I assume so

@compnerd
Copy link
Member Author

compnerd commented Apr 5, 2023

@DougGregor yea, we will >.<

@compnerd
Copy link
Member Author

compnerd commented Apr 5, 2023

@swift-ci please test macOS platform

@MaxDesiatov
Copy link
Contributor

@swift-ci test macos

@compnerd compnerd force-pushed the public-your-private branch from 2b90205 to ce001f7 Compare April 5, 2023 19:40
@compnerd
Copy link
Member Author

compnerd commented Apr 5, 2023

@swift-ci please test

1 similar comment
@compnerd
Copy link
Member Author

compnerd commented Apr 5, 2023

@swift-ci please test

@MaxDesiatov
Copy link
Contributor

@swift-ci test macos

@compnerd
Copy link
Member Author

compnerd commented Apr 6, 2023

:sigh:

FAIL: Swift(macosx-x86_64) :: IRGen/serialised-pwt-afp.swift (2799 of 16207)
******************** TEST 'Swift(macosx-x86_64) :: IRGen/serialised-pwt-afp.swift' FAILED ********************
Script:
--
: 'RUN: at line 1';   rm -rf "/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/IRGen/Output/serialised-pwt-afp.swift.tmp" && mkdir -p "/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/IRGen/Output/serialised-pwt-afp.swift.tmp"
: 'RUN: at line 2';   /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/swift-macosx-x86_64/bin/swift-frontend -target x86_64-apple-macosx10.13  -module-cache-path /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache -sdk '/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk'  -swift-version 4  -define-availability 'SwiftStdlib 9999:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999' -define-availability 'SwiftStdlib 5.0:macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2' -define-availability 'SwiftStdlib 5.1:macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0' -define-availability 'SwiftStdlib 5.2:macOS 10.15.4, iOS 13.4, watchOS 6.2, tvOS 13.4' -define-availability 'SwiftStdlib 5.3:macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0' -define-availability 'SwiftStdlib 5.4:macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5' -define-availability 'SwiftStdlib 5.5:macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0' -define-availability 'SwiftStdlib 5.6:macOS 12.3, iOS 15.4, watchOS 8.5, tvOS 15.4' -define-availability 'SwiftStdlib 5.7:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0' -define-availability 'SwiftStdlib 5.8:macOS 13.3, iOS 16.4, watchOS 9.4, tvOS 16.4' -define-availability 'SwiftStdlib 5.9:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999' -typo-correction-limit 10  -emit-module -emit-module-path /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/IRGen/Output/serialised-pwt-afp.swift.tmp/P.swiftmodule -parse-as-library -module-name P -DP /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/test/IRGen/serialised-pwt-afp.swift
: 'RUN: at line 3';   /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/swift-macosx-x86_64/bin/swift-frontend -target x86_64-apple-macosx10.13  -module-cache-path /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache -sdk '/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk'  -swift-version 4  -define-availability 'SwiftStdlib 9999:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999' -define-availability 'SwiftStdlib 5.0:macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2' -define-availability 'SwiftStdlib 5.1:macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0' -define-availability 'SwiftStdlib 5.2:macOS 10.15.4, iOS 13.4, watchOS 6.2, tvOS 13.4' -define-availability 'SwiftStdlib 5.3:macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0' -define-availability 'SwiftStdlib 5.4:macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5' -define-availability 'SwiftStdlib 5.5:macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0' -define-availability 'SwiftStdlib 5.6:macOS 12.3, iOS 15.4, watchOS 8.5, tvOS 15.4' -define-availability 'SwiftStdlib 5.7:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0' -define-availability 'SwiftStdlib 5.8:macOS 13.3, iOS 16.4, watchOS 9.4, tvOS 16.4' -define-availability 'SwiftStdlib 5.9:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999' -typo-correction-limit 10  -I/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/IRGen/Output/serialised-pwt-afp.swift.tmp -parse-as-library -module-name Q -c /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/test/IRGen/serialised-pwt-afp.swift -o /dev/null -validate-tbd-against-ir=missing
--
Exit Code: 1

Command Output (stderr):
--
/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/test/IRGen/serialised-pwt-afp.swift:9:12: error: concurrency is only available in macOS 10.15.0 or newer
  func f() async
           ^
/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/test/IRGen/serialised-pwt-afp.swift:9:8: note: add @available attribute to enclosing instance method
  func f() async
       ^
/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/test/IRGen/serialised-pwt-afp.swift:8:17: note: add @available attribute to enclosing protocol
public protocol P {
                ^

--

However, the test is marked with: // REQUIRES: concurrency

@etcwilde
Copy link
Contributor

etcwilde commented Apr 6, 2023

REQUIRES: CONCURRENCY describes where we’re building, which supports concurrency, not what the test is targeting. The test is targeting 10.13, so it fails. I think you can add -disable-availability-checking to the test and get it working.

When a protocol witness thunk is formed to a serialised protocol
containing an `async` function, the async function pointer to the
conformance needs to be made public due to a SIL Verifier check failure
(reference to a non-fragile function from within a public fragile
function).  Add a stop-gap solution of rolling an extra emission for a
private symbol as a public symbol to avoid the error (the underlying
issue has been open for ~6y and counting as of this commit).

This was identified by swift-package-manager (swiftlang#64900).

Thanks to @DougGregor and @aschwaighofer for the discussion on this!
@compnerd compnerd force-pushed the public-your-private branch from ce001f7 to 0301e03 Compare April 6, 2023 16:04
@compnerd
Copy link
Member Author

compnerd commented Apr 6, 2023

@swift-ci please test

@tshortli
Copy link
Contributor

tshortli commented Apr 6, 2023

REQUIRES: CONCURRENCY describes where we’re building, which supports concurrency, not what the test is targeting. The test is targeting 10.13, so it fails. I think you can add -disable-availability-checking to the test and get it working.

This is not a request for a change to this PR but I just want to note that in most tests I'd prefer it if folks just added the right availability to declarations that depend on concurrency so that we're testing that availability checking behaves correctly. The declarations just need @available(SwiftStdlib 5.1, *).

@compnerd
Copy link
Member Author

compnerd commented Apr 6, 2023

@swift-ci please test macOS platform

@compnerd
Copy link
Member Author

compnerd commented Apr 7, 2023

beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk/usr/lib/swift/Foundation.swiftmodule/x86_64-apple-macos.swiftinterface:4:8: error: no such module 'Combine'
import Combine
       ^
/private/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/Miscellaneous_AtMainSupport.EEVf1W/Miscellaneous_AtMainSupport/Package.swift:1:8: error: failed to build module 'Foundation'; this SDK is not supported by the compiler (the SDK is built with 'Apple Swift version 5.5 (swiftlang-1300.0.27.6 clang-1300.0.27.2)', while this compiler is 'Apple Swift version 5.9-dev (LLVM d3767c21e9410d7, Swift 2cd4c2186cadec1)'). Please select a toolchain which matches the SDK.
import Foundation
       ^

@compnerd
Copy link
Member Author

compnerd commented Apr 7, 2023

CC: @shahmishal

@compnerd
Copy link
Member Author

compnerd commented Apr 7, 2023

@swift-ci please test macOS platform

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.

6 participants