Skip to content

Add += operator for Trivia #612

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
Aug 23, 2022
Merged

Add += operator for Trivia #612

merged 1 commit into from
Aug 23, 2022

Conversation

fwcd
Copy link
Member

@fwcd fwcd commented Aug 19, 2022

A small convenience that makes it more convenient to construct Trivia imperatively.

@fwcd fwcd requested a review from ahoppen as a code owner August 19, 2022 19:03
@fwcd
Copy link
Member Author

fwcd commented Aug 19, 2022

@swift-ci please test

@fwcd fwcd force-pushed the tokens-plus-assign branch from 6932ed3 to 27c3404 Compare August 19, 2022 19:15
@fwcd
Copy link
Member Author

fwcd commented Aug 19, 2022

@swift-ci please test

Comment on lines 218 to 224
/// Concatenates two collections of `Trivia` into the left-hand side.
public func +=(lhs: inout Trivia, rhs: Trivia) {
lhs = lhs + rhs
}

Copy link
Member

Choose a reason for hiding this comment

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

I have a vague memory that from a type checking performance perspective, it’s better to implement these as static methods inside the Trivia type itself.

Suggested change
/// Concatenates two collections of `Trivia` into the left-hand side.
public func +=(lhs: inout Trivia, rhs: Trivia) {
lhs = lhs + rhs
}
extension Trivia {
/// Concatenates two collections of `Trivia` into the left-hand side.
public static func +=(lhs: inout Trivia, rhs: Trivia) {
lhs = lhs + rhs
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Does this apply to the + operator too?

Copy link
Contributor

Choose a reason for hiding this comment

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

It used to, but we removed the designated types optimization that supported it. Now it should mostly a hygiene thing, and from that perspective I agree with Alex that this should be in extension context.

@fwcd fwcd force-pushed the tokens-plus-assign branch from 27c3404 to 3b1d6c8 Compare August 20, 2022 21:35
@fwcd
Copy link
Member Author

fwcd commented Aug 20, 2022

@swift-ci please test

@fwcd fwcd merged commit 79e86b5 into swiftlang:main Aug 23, 2022
@fwcd fwcd deleted the tokens-plus-assign branch August 23, 2022 09:59
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