-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Added protocol to support CVarArg objects that need to be retained #32311
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
Added protocol to support CVarArg objects that need to be retained #32311
Conversation
This is desirable for source compatibility between Darwin Foundation and corelib Foundation. |
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.
Since this is affecting public entry points, we either need to add availability for Darwin platforms or we need to make the new entries only available on platforms where there is no Objective-C runtime. I suspect the latter is the better choice?
@Molanda I would recommend only |
Thank you for the comments. I will make the changes sometime this weekend. |
I have made the recommended changes so the retaining object protocol is only used when the ObjC runtime is not available. |
@swift-ci please test |
@lorentey have any opinions on this? |
Just checking in to see if anything additional is needed on this one. |
@DougGregor @lorentey any more feedback? This is highly desired for master. |
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.
Looks good to me!
It would be desirable not to autorelease on Apple platforms, either. To do that, we'd probably want to augment the retainer
array with inline space for a couple object references (to prevent allocations in the common case), and I'm 99% sure we'd also want to remove @inlinable
from all builder methods. (Exposing the vararg builder's class members in the ABI was a regrettable step.)
To fix string formatting using
%@
and aString
argument, theCVarArg
protocol needs to be implemented onString
. To do this,String
requires anNSString
to return aCFString
which is what is finally used by the formatting.However, without an autorelease pool (as is the case with most non-Apple platforms),
NSString
will get destroyed once out of scope of_cVarArgEncoding
.This pull request adds a new protocol to support retaining such objects without requiring an autorelease pool.
Please see Foundation PR #2821 and this post for more details.
Resolves SR-957.