Skip to content

[5.8][stdlib] Expose index rounding entry points #62860

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 2 commits into from
Jan 6, 2023

Conversation

lorentey
Copy link
Member

@lorentey lorentey commented Jan 5, 2023

Cherry picked from #62798.

These simply expose the preexisting algorithms that String views use to implicitly round indices down to the nearest valid index within the view. (Or, in the case of the encoding views, rounding down to the nearest scalar boundary.)

Being able to do this as a standalone, explicit, efficient operation is crucial when implementing some String algorithms that need to work with arbitrary string indices.

This PR exposes these operations as unstable, underscored entry points. Ideally these would get upgraded to public API soon, but first I'd like to prove these have the right semantics by experimenting with using them in their underscored form.

rdar://101652714

… views

These simply expose the preexisting internal
`_StringGuts.validate*Index` functions that indexing operations
use to implicitly round indices down to the nearest valid index. (Or, in the case of the encoding views, the nearest scalar boundary.)

Being able to do this as a standalone, explicit, efficient operation
is crucial when implementing some `String` algorithms that need to
work with arbitrary indices.
@lorentey
Copy link
Member Author

lorentey commented Jan 5, 2023

@swift-ci test

Copy link
Contributor

@stephentyrone stephentyrone left a comment

Choose a reason for hiding this comment

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

This all looks basically fine; can you explain what considerations lead to marking them all @aeic?

@lorentey
Copy link
Member Author

lorentey commented Jan 5, 2023

The implementations for rounding indices within the Unicode scalar, UTF-8 and UTF-16 views were already fully transparent within the .swiftinterface. The new entry points simply make these accessible to clients outside the stdlib, and it felt better not to limit their availability.

An alternative way to achieve the same would be to use @_backDeploy @inlinable. As of 080d850, this is now a supported combination, but I don't feel comfortable using it yet in the stdlib. I think it could be an option on main, though.

The index rounding entry points for character views weren't previously exposed, so the new methods for those views come with 5.8 availability.

@stephentyrone
Copy link
Contributor

Yeah, to be clear, I have no objection to marking them aEIC, I just want to understand the rationale.

@lorentey
Copy link
Member Author

lorentey commented Jan 5, 2023

I don't really have a strong opinion here -- @_aeic is simply my default choice in this case.

Letting index rounding deploy back to earlier stdlibs when possible is the most developer-friendly option. The rounding implementations are already inlined into clients via the regular indexing APIs, so emitting them has no real client-side code size cost. (This also makes the inability for stdlib updates to change the behavior of these entry points in existing binaries largely moot -- that concern is valid, but it applies far more strongly to the normal index APIs.) Marking the new entry points aeic also leaves the door open to (eventually) replace these with non-underscored public API, without having to deal with an ABI symbol -- if we choose to go that way.

@lorentey lorentey merged commit d108a04 into swiftlang:release/5.8 Jan 6, 2023
@lorentey lorentey deleted the string-index-rounding-5.8 branch January 6, 2023 00:03
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.8 labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants