Skip to content
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

Annotate marker protocols with @_marker #1497

Closed
wants to merge 1 commit into from

Conversation

mjburghard
Copy link
Contributor

Apple introduced the @_marker attribute to enforce a property at compile time without adding any runtime information.
Swift-protobuf contains two protocols, _MessageBase and ProtobufAPIVersion_3 , that fulfill the required restrictions and can be annotated with the attribute.
The main motivation for this change is the reduction in code size. In our case this result in a 80 kB decrease.

@_marker is prefixed with an underscore, so usage is discouraged in general. I still see a benefit adding the attribute to the two protocols. In the worst case, it needs to be removed when a future Swift version is released and the code size regresses.

More information about the semantics can be found here and here. The attribute was introduced as part of Swift 5.7.

@@ -115,7 +115,7 @@ public protocol Message: _MessageBase {
#if DEBUG
public protocol _MessageBase: Sendable, CustomDebugStringConvertible {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not also in DEBUG builds?

Copy link
Contributor Author

@mjburghard mjburghard Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They cannot inherit from non-marker protocols.

CustomDebugStringConvertible is not a marker protocol.

Copy link
Member

@FranzBusch FranzBusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should add this for the general case since it uses an underscored attribute. This can change at any point in time and is not a stable feature.

@tbkka
Copy link
Collaborator

tbkka commented Nov 8, 2023

_MessageBase and the similar _AnyExtensionField protocol were added in #1246 to provide a way to control whether the main Message and AnyExtensionField protocols conform to CustomDebugStringConvertible. Is there another way to achieve this without adding those new intermediate protocols? (Not having those protocols at all would be even better than making them @_marker.)

@mjburghard
Copy link
Contributor Author

I think I found a way that does not require @_marker, but want to verify that it achieves the same size reduction tomorrow morning.

@mjburghard
Copy link
Contributor Author

I found a way that achieves the same without using @_marker. I close this PR. Thanks for your suggestions.

@mjburghard mjburghard closed this Nov 9, 2023
@tbkka
Copy link
Collaborator

tbkka commented Nov 9, 2023

Can you provide any details? I wonder if whatever you found is something we should be recommending to other people?

@thomasvl
Copy link
Collaborator

thomasvl commented Nov 9, 2023

Can you provide any details? I wonder if whatever you found is something we should be recommending to other people?

I believe the answer is #1498.

@tbkka
Copy link
Collaborator

tbkka commented Nov 9, 2023

Ah. I see #1498 now.

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.

4 participants