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

Provide a C++ interface to the collator #2218

Closed
hsivonen opened this issue Jul 20, 2022 · 2 comments
Closed

Provide a C++ interface to the collator #2218

hsivonen opened this issue Jul 20, 2022 · 2 comments
Assignees
Labels
C-collator Component: Collation, normalization C-ffi-infra Component: Diplomat, horizontal FFI

Comments

@hsivonen
Copy link
Member

Currently, the collator isn't exposed to C++ via Diplomat. It should be.

Apart from the constructor, the Collator object has three public methods in Rust:

  • compare() (takes guaranteed-valid UTF-8)
  • compare_utf8() (takes potentially-invalid UTF-8)
  • compare_utf16() (takes potentially-invalid UTF-16)

I expect these to be exposed to C++ as follows:

  • C++ compare() taking std::u16string_view: Rust compare_utf16()
  • C++ compare() taking std::string_view: Rust compare_utf8()
  • C++ compare_unsafe() taking std::string_view: Rust compare() with documentation note that the call is UB unless the C++ caller has ensured UTF-8 validity.

(Once we support C++20, there should be std::u8string_view overloads for the same things that have std::string_view versions.)

In Rust, these return Ordering. It's a bit unclear if they should return int with values -1, 0, 1 or something fancier in C++.

@hsivonen hsivonen added C-ffi-infra Component: Diplomat, horizontal FFI C-collator Component: Collation, normalization labels Jul 20, 2022
@hsivonen hsivonen added this to the ICU4X 1.0 (Features) milestone Jul 20, 2022
@hsivonen
Copy link
Member Author

hsivonen commented Aug 26, 2022

C++ compare() taking std::string_view: Rust compare_utf8()

To get Diplomat to use std::string_view in C++, the argument type of the method on the #[diplomat::opaque] ICU4XCollator should be &str, which should immediately get as_bytes() called on it.

C++ compare() taking std::u16string_view: Rust compare_utf16()

This requires more Diplomat configurability.

@echeran
Copy link
Contributor

echeran commented Sep 7, 2022

Fixed by #2498. That covers adding Collator FFI via Diplomat. The remaining details in the description of this issue -- about customizing the APIs produced in C++ -- are a part of the plan outlined in #2520.

@echeran echeran closed this as completed Sep 7, 2022
@echeran echeran assigned echeran and unassigned pdogr Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-collator Component: Collation, normalization C-ffi-infra Component: Diplomat, horizontal FFI
Projects
None yet
Development

No branches or pull requests

5 participants