Copilot commented on code in PR #873:
URL: https://github.com/apache/iceberg-cpp/pull/873#discussion_r3915385651
##########
src/iceberg/manifest/manifest_reader.cc:
##########
@@ -689,8 +690,147 @@ Result<std::shared_ptr<Schema>>
ProjectSchema(std::shared_ptr<Schema> schema,
return schema;
}
+template <typename T>
+class VectorIterator final : public Iterator<T> {
+ public:
+ explicit VectorIterator(std::vector<T> values) : values_(std::move(values))
{}
+
+ Result<std::optional<T>> Next() override {
+ if (next_ == values_.size()) {
+ return std::nullopt;
+ }
+ return std::optional<T>{std::move(values_[next_++])};
Review Comment:
`VectorIterator::Next()` unconditionally `std::move`s out of `values_`,
which fails to compile for copy-only `T` (deleted move ctor). Since
`Iterator::ToVector()` explicitly supports copy-only `T`, this adapter should
too. Fix by conditionally copying when `T` is not move-constructible (e.g., use
an `if constexpr` on `std::is_move_constructible_v<T>` and otherwise return
`values_[next_++]` by copy).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
