-
Notifications
You must be signed in to change notification settings - Fork 194
FFI: get scalars from arrays #8896
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
Open
myrrc
wants to merge
2
commits into
develop
Choose a base branch
from
myrrc/ffi-getter-session
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,20 +26,93 @@ class Scalar { | |
| bool is_null() const; | ||
| DataType dtype() const; | ||
|
|
||
| /** | ||
| * Read scalar's value. | ||
| * | ||
| * Supported types are bool, primitives, std::string_view, and BinaryView. | ||
| * | ||
| * std::string_view and BinaryView returned borrow from scalar and stay | ||
| * valid while scalar is valid. | ||
| * | ||
| * Throws if scalar type does not match T or value is Null. | ||
| */ | ||
| template <element_type T> | ||
| T get() const; | ||
|
|
||
| /** | ||
| * Read a decimal scalar's unscaled value. | ||
| * int8/16/32/64_t are supported. | ||
| * Throws if scalar is not a decimal, is Null, or value does not fit in T. | ||
| */ | ||
| template <primitive_type T> | ||
| T get_decimal() const; | ||
|
|
||
| private: | ||
| friend struct detail::Access; | ||
| explicit Scalar(vx_scalar *owned) : handle_(owned) { | ||
| explicit Scalar(const vx_scalar *owned) : handle_(owned) { | ||
| } | ||
| vx_scalar *release() && { | ||
| const vx_scalar *release() && { | ||
| return handle_.release(); | ||
| } | ||
|
|
||
| struct Deleter { | ||
| void operator()(vx_scalar *ptr) const noexcept; | ||
| void operator()(const vx_scalar *ptr) const noexcept; | ||
| }; | ||
| std::unique_ptr<vx_scalar, Deleter> handle_; | ||
| std::unique_ptr<const vx_scalar, Deleter> handle_; | ||
| }; | ||
|
|
||
| template <element_type T> | ||
| T Scalar::get() const { | ||
| const vx_scalar *h = handle_.get(); | ||
| if constexpr (std::is_same_v<T, bool>) { | ||
| return vx_scalar_get_bool(h); | ||
| } else if constexpr (std::is_same_v<T, std::string_view>) { | ||
| vx_view v = vx_scalar_get_utf8(h); | ||
| return std::string_view(v.ptr, v.len); | ||
| } else if constexpr (std::is_same_v<T, BinaryView>) { | ||
| vx_view v = vx_scalar_get_binary(h); | ||
| return BinaryView(reinterpret_cast<const std::byte *>(v.ptr), v.len); | ||
| } else if constexpr (std::is_same_v<T, uint8_t>) { | ||
| return vx_scalar_get_u8(h); | ||
| } else if constexpr (std::is_same_v<T, uint16_t>) { | ||
| return vx_scalar_get_u16(h); | ||
| } else if constexpr (std::is_same_v<T, uint32_t>) { | ||
| return vx_scalar_get_u32(h); | ||
| } else if constexpr (std::is_same_v<T, uint64_t>) { | ||
| return vx_scalar_get_u64(h); | ||
| } else if constexpr (std::is_same_v<T, int8_t>) { | ||
| return vx_scalar_get_i8(h); | ||
| } else if constexpr (std::is_same_v<T, int16_t>) { | ||
| return vx_scalar_get_i16(h); | ||
| } else if constexpr (std::is_same_v<T, int32_t>) { | ||
| return vx_scalar_get_i32(h); | ||
| } else if constexpr (std::is_same_v<T, int64_t>) { | ||
| return vx_scalar_get_i64(h); | ||
| } else if constexpr (std::is_same_v<T, float>) { | ||
| return vx_scalar_get_f32(h); | ||
| } else if constexpr (std::is_same_v<T, double>) { | ||
| return vx_scalar_get_f64(h); | ||
| } else { | ||
| static_assert(false, "f16 scalar get is not supported"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we not support f16? I mean we do support constructing f16 arrays or scalars, but we can't get a scalar? |
||
| } | ||
| } | ||
|
|
||
| template <primitive_type T> | ||
| T Scalar::get_decimal() const { | ||
| const vx_scalar *h = handle_.get(); | ||
| if constexpr (std::is_same_v<T, int8_t>) { | ||
| return vx_scalar_get_decimal_i8(h); | ||
| } else if constexpr (std::is_same_v<T, int16_t>) { | ||
| return vx_scalar_get_decimal_i16(h); | ||
| } else if constexpr (std::is_same_v<T, int32_t>) { | ||
| return vx_scalar_get_decimal_i32(h); | ||
| } else if constexpr (std::is_same_v<T, int64_t>) { | ||
| return vx_scalar_get_decimal_i64(h); | ||
| } else { | ||
| static_assert(false, "unsupported decimal scalar get type"); | ||
| } | ||
| } | ||
|
|
||
| namespace detail { | ||
| vx_scalar *make_bool(bool value, bool nullable); | ||
| vx_scalar *make_primitive(vx_ptype ptype, const void *value, bool nullable); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
we say this method throws here but it does actually panic right? Shall we validate the dtype and validity and throw so we do what we say?