gemini-code-assist[bot] commented on code in PR #218:
URL: https://github.com/apache/tvm-ffi/pull/218#discussion_r2485851693
##########
include/tvm/ffi/container/container_details.h:
##########
@@ -175,9 +175,9 @@ template <typename Converter, typename TIter>
class IterAdapter {
public:
using difference_type = typename
std::iterator_traits<TIter>::difference_type;
- using value_type = typename Converter::ResultType;
- using pointer = typename Converter::ResultType*;
- using reference = typename Converter::ResultType&;
+ using value_type = const typename Converter::ResultType;
+ using pointer = const typename Converter::ResultType*;
+ using reference = const typename Converter::ResultType;
Review Comment:
While this change correctly makes `operator*`'s return type consistent with
`reference`, the `value_type` should be a non-const, non-reference type
according to C++ iterator requirements. For example, `std::vector<const
int>::iterator` has `value_type` as `int`, not `const int`.
`value_type` should be `typename Converter::ResultType`.
```c
using value_type = typename Converter::ResultType;
using pointer = const typename Converter::ResultType*;
using reference = const typename Converter::ResultType;
```
##########
include/tvm/ffi/container/container_details.h:
##########
@@ -175,9 +175,9 @@ template <typename Converter, typename TIter>
class IterAdapter {
public:
using difference_type = typename
std::iterator_traits<TIter>::difference_type;
- using value_type = typename Converter::ResultType;
- using pointer = typename Converter::ResultType*;
- using reference = typename Converter::ResultType&;
+ using value_type = const typename Converter::ResultType;
+ using pointer = const typename Converter::ResultType*;
+ using reference = const typename Converter::ResultType;
using iterator_category = typename
std::iterator_traits<TIter>::iterator_category;
Review Comment:
The `iterator_category` is inherited from `TIter`. However, since
`operator*` returns a temporary value (a prvalue), this iterator can only model
a `LegacyInputIterator`. If `TIter` is a `ForwardIterator` or stronger,
`IterAdapter` will advertise a stronger category than it actually supports.
This can lead to undefined behavior with STL algorithms that require
`ForwardIterator` or stronger capabilities (e.g., multi-pass algorithms).
To be fully correct, the `iterator_category` should be
`std::input_iterator_tag`.
```c
using iterator_category = std::input_iterator_tag;
```
--
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]
