Skip to content

[Concurrency] Emit async methods in ObjC protocols once. #36822

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

varungandhi-apple
Copy link
Contributor

Previously, the method table would contain duplicate copies due to the
ProtocolDecl carrying a completionHandler-based version of the method,
as well as the async version of the method.

Fixes rdar://76192003.

@varungandhi-apple
Copy link
Contributor Author

@swift-ci smoke test

@varungandhi-apple
Copy link
Contributor Author

varungandhi-apple commented Apr 8, 2021

There is a bug in the current implementation, this will skip methods in @objc protocols written in Swift, not producing an encoding string at all. Fixed by adding a hasClangNode() check.

import Foundation; @objc protocol MyProto { func coolbeans() async -> String }; MyProto.self

// Before
L___unnamed_1:
	.asciz	"_TtP4main7MyProto_"

	.section	__TEXT,__objc_methname,cstring_literals
"L_selector_data(coolbeansWithCompletionHandler:)":
	.asciz	"coolbeansWithCompletionHandler:"

	.section	__TEXT,__cstring,cstring_literals
	.p2align	4
L___unnamed_2:
	.asciz	"v24@0:8@?<v@?@\"NSString\">16"

	.section	__DATA,__objc_data
	.p2align	3
__PROTOCOL_INSTANCE_METHODS__TtP4main7MyProto_:
	.long	24
	.long	1
	.quad	"L_selector_data(coolbeansWithCompletionHandler:)"
	.quad	L___unnamed_2
	.quad	0

	.section	__DATA,__objc_const
	.p2align	3
__PROTOCOL_METHOD_TYPES__TtP4main7MyProto_:
	.quad	L___unnamed_2

// After
L___unnamed_1:
	.asciz	"_TtP4main7MyProto_"

Previously, the method table would contain duplicate copies due to the
ProtocolDecl carrying a completionHandler-based version of the method,
as well as the async version of the method.

Fixes rdar://76192003.
@varungandhi-apple varungandhi-apple force-pushed the vg-async-method-emit-once branch from 2e8a80a to b6ed507 Compare April 8, 2021 23:26
@varungandhi-apple
Copy link
Contributor Author

@swift-ci smoke test

@varungandhi-apple varungandhi-apple merged commit 0ffa5f8 into swiftlang:main Apr 9, 2021
@atrick
Copy link
Contributor

atrick commented Apr 9, 2021

@varungandhi-apple smoke test only means this blocks @swift-ci test

Concurrency/objc_async_protocol_irgen.swift:15:16: error: CHECK-SAME: is not on the same line as the previous match
// CHECK-SAME: align 8
^
:55:43: note: 'next' match was here
%2 = load atomic i64, i64* %1 monotonic, align 8
^
:28:251: note: previous match ended here
@_PROTOCOL_INSTANCE_METHODS_MyAsyncProtocol = internal constant { i32, i32, [1 x { i8*, i8*, i8* }] } { i32 12, i32 1, [1 x { i8*, i8*, i8* }] [{ i8*, i8*, i8* } { i8* getelementptr inbounds ([15 x i8], [15 x i8]* @"\01L_selector_data(myAsyncMethod:)", i32 0, i32 0), i8* getelementptr inbounds ([11 x i8], [11 x i8]* @1, i32 0, i32 0), i8* null }] }, section "__DATA, __objc_data", align 4
^

Input file:
Check file: /Users/atrick/s/sfix/swift/test/Concurrency/objc_async_protocol_irgen.swift

-dump-input=help explains the following input dump.

Input was:
<<<<<<
.
.
.
50:
51: ; Function Attrs: noinline nounwind readnone
52: define linkonce_odr hidden %swift.type* @__swift_instantiateConcreteTypeFromMangledName({ i32, i32 }* %0) #1 {
53: entry:
54: %1 = bitcast { i32, i32 }* %0 to i64*
55: %2 = load atomic i64, i64* %1 monotonic, align 8
same:15 !~~~~~~ error: match on wrong line
56: %3 = icmp slt i64 %2, 0
57: %4 = call i1 @llvm.expect.i1(i1 %3, i1 false)
58: br i1 %4, label %9, label %5
59:
60: 5: ; preds = %9, %entry

@atrick
Copy link
Contributor

atrick commented Apr 9, 2021

32-bit fix: #36840

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.

3 participants