Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao merged PR #443: URL: https://github.com/apache/tvm-ffi/pull/443 -- 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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2805960312
##
include/tvm/ffi/container/list.h:
##
@@ -0,0 +1,523 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ffi/container/list.h
+ * \brief Mutable list type.
+ *
+ * tvm::ffi::List is an erased mutable sequence container.
+ */
+#ifndef TVM_FFI_CONTAINER_LIST_H_
+#define TVM_FFI_CONTAINER_LIST_H_
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace tvm {
+namespace ffi {
+
+/*! \brief List node content in list */
+class ListObj : public SeqBaseObj {
+ public:
+ /*!
+ * \brief Constructs a container with n elements. Each element is a copy of
val
+ * \param n The size of the container
+ * \param val The init value
+ * \return Ref-counted ListObj requested
+ */
+ static ObjectPtr CreateRepeated(int64_t n, const Any& val) {
+ObjectPtr p = ListObj::Empty(n);
+Any* itr = p->MutableBegin();
+for (int64_t& i = p->TVMFFISeqCell::size = 0; i < n; ++i) {
+ new (itr++) Any(val);
+}
+return p;
+ }
+
+ /// \cond Doxygen_Suppress
+ static constexpr const int32_t _type_index = TypeIndex::kTVMFFIList;
+ static const constexpr bool _type_final = true;
+ TVM_FFI_DECLARE_OBJECT_INFO_STATIC(StaticTypeKey::kTVMFFIList, ListObj,
Object);
+ /// \endcond
+
+ private:
+ /*!
+ * \brief Ensure the list has at least n slots.
+ * \param n The lower bound of required capacity.
+ */
+ void Reserve(int64_t n) {
+if (n <= TVMFFISeqCell::capacity) {
+ return;
+}
+Any* old_data = MutableBegin();
+Any* new_data = static_cast(::operator new(sizeof(Any) *
static_cast(n)));
+for (int64_t i = 0; i < TVMFFISeqCell::size; ++i) {
Review Comment:
updated
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2805953527
##
src/ffi/extra/structural_hash.cc:
##
@@ -185,11 +190,25 @@ class StructuralHashHandler {
}
// NOLINTNEXTLINE(performance-unnecessary-value-param)
- uint64_t HashArray(Array arr) {
-uint64_t hash_value = details::StableHashCombine(arr->GetTypeKeyHash(),
arr.size());
-for (const auto& elem : arr) {
+ uint64_t HashArray(Array arr) { return HashSequence(std::move(arr)); }
+
+ // NOLINTNEXTLINE(performance-unnecessary-value-param)
+ uint64_t HashList(List list) { return HashSequence(std::move(list)); }
+
+ template
+ // NOLINTNEXTLINE(performance-unnecessary-value-param)
+ uint64_t HashSequence(SeqType seq) {
+const Object* obj_ptr = seq.get();
Review Comment:
removed
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2805952669
##
src/ffi/extra/structural_equal.cc:
##
@@ -302,6 +309,33 @@ class StructEqualHandler {
// NOLINTNEXTLINE(performance-unnecessary-value-param)
bool CompareArray(ffi::Array lhs, ffi::Array rhs) {
+return CompareSequence(std::move(lhs), std::move(rhs));
+ }
+
+ // NOLINTNEXTLINE(performance-unnecessary-value-param)
+ bool CompareList(ffi::List lhs, ffi::List rhs) {
+return CompareSequence(std::move(lhs), std::move(rhs));
+ }
+
+ template
+ // NOLINTNEXTLINE(performance-unnecessary-value-param)
+ bool CompareSequence(SeqType lhs, SeqType rhs) {
+const Object* lhs_ptr = lhs.get();
+const Object* rhs_ptr = rhs.get();
+auto pair = std::make_pair(lhs_ptr, rhs_ptr);
+if (active_sequence_pairs_.count(pair)) {
Review Comment:
removed
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
tqchen commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2805843597
##
src/ffi/extra/structural_equal.cc:
##
@@ -302,6 +309,33 @@ class StructEqualHandler {
// NOLINTNEXTLINE(performance-unnecessary-value-param)
bool CompareArray(ffi::Array lhs, ffi::Array rhs) {
+return CompareSequence(std::move(lhs), std::move(rhs));
+ }
+
+ // NOLINTNEXTLINE(performance-unnecessary-value-param)
+ bool CompareList(ffi::List lhs, ffi::List rhs) {
+return CompareSequence(std::move(lhs), std::move(rhs));
+ }
+
+ template
+ // NOLINTNEXTLINE(performance-unnecessary-value-param)
+ bool CompareSequence(SeqType lhs, SeqType rhs) {
+const Object* lhs_ptr = lhs.get();
+const Object* rhs_ptr = rhs.get();
+auto pair = std::make_pair(lhs_ptr, rhs_ptr);
+if (active_sequence_pairs_.count(pair)) {
Review Comment:
main reason is for sequal/hash there can be more kind of cycles, so only
doing it for list is a bit limited, and usually the usecase is for non-cycle,
so keeping things simple here should be good. we can use cycle detection in
json dumps
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
tqchen commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2805334781
##
include/tvm/ffi/container/list.h:
##
@@ -0,0 +1,523 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ffi/container/list.h
+ * \brief Mutable list type.
+ *
+ * tvm::ffi::List is an erased mutable sequence container.
+ */
+#ifndef TVM_FFI_CONTAINER_LIST_H_
+#define TVM_FFI_CONTAINER_LIST_H_
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace tvm {
+namespace ffi {
+
+/*! \brief List node content in list */
+class ListObj : public SeqBaseObj {
+ public:
+ /*!
+ * \brief Constructs a container with n elements. Each element is a copy of
val
+ * \param n The size of the container
+ * \param val The init value
+ * \return Ref-counted ListObj requested
+ */
+ static ObjectPtr CreateRepeated(int64_t n, const Any& val) {
+ObjectPtr p = ListObj::Empty(n);
+Any* itr = p->MutableBegin();
+for (int64_t& i = p->TVMFFISeqCell::size = 0; i < n; ++i) {
+ new (itr++) Any(val);
+}
+return p;
+ }
+
+ /// \cond Doxygen_Suppress
+ static constexpr const int32_t _type_index = TypeIndex::kTVMFFIList;
+ static const constexpr bool _type_final = true;
+ TVM_FFI_DECLARE_OBJECT_INFO_STATIC(StaticTypeKey::kTVMFFIList, ListObj,
Object);
+ /// \endcond
+
+ private:
+ /*!
+ * \brief Ensure the list has at least n slots.
+ * \param n The lower bound of required capacity.
+ */
+ void Reserve(int64_t n) {
+if (n <= TVMFFISeqCell::capacity) {
+ return;
+}
+Any* old_data = MutableBegin();
+Any* new_data = static_cast(::operator new(sizeof(Any) *
static_cast(n)));
+for (int64_t i = 0; i < TVMFFISeqCell::size; ++i) {
Review Comment:
i think so, we should document via comment
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2805246768
##
include/tvm/ffi/container/seq_base.h:
##
@@ -0,0 +1,357 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ffi/container/seq_base.h
+ * \brief Base class for sequence containers (Array, List).
+ */
+#ifndef TVM_FFI_CONTAINER_SEQ_BASE_H_
+#define TVM_FFI_CONTAINER_SEQ_BASE_H_
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+namespace tvm {
+namespace ffi {
+
+/*!
+ * \brief Base class for sequence containers (ArrayObj, ListObj).
+ *
+ * SeqBaseObj is transparent to the FFI type system (no type index),
+ * following the same pattern as BytesObjBase.
+ */
+class SeqBaseObj : public Object, protected TVMFFISeqCell {
+ public:
+ SeqBaseObj() {
+data = nullptr;
+TVMFFISeqCell::size = 0;
+TVMFFISeqCell::capacity = 0;
+data_deleter = nullptr;
+ }
+
+ ~SeqBaseObj() {
+Any* begin = MutableBegin();
+for (int64_t i = 0; i < TVMFFISeqCell::size; ++i) {
+ (begin + i)->Any::~Any();
+}
+if (data_deleter != nullptr) {
+ data_deleter(data);
+}
+ }
+
+ /*! \return The size of the sequence */
+ size_t size() const { return static_cast(TVMFFISeqCell::size); }
+
+ /*! \return The capacity of the sequence */
+ size_t capacity() const { return
static_cast(TVMFFISeqCell::capacity); }
+
+ /*! \return Whether the sequence is empty */
+ bool empty() const { return TVMFFISeqCell::size == 0; }
+
+ /*!
+ * \brief Read i-th element from the sequence.
+ * \param i The index
+ * \return the i-th element.
+ */
+ const Any& at(int64_t i) const { return this->operator[](i); }
+
+ /*!
+ * \brief Read i-th element from the sequence.
+ * \param i The index
+ * \return the i-th element.
+ */
+ const Any& operator[](int64_t i) const {
+if (i < 0 || i >= TVMFFISeqCell::size) {
+ TVM_FFI_THROW(IndexError) << "Index " << i << " out of bounds " <<
TVMFFISeqCell::size;
+}
+return static_cast(data)[i];
+ }
+
+ /*! \return The first element */
+ const Any& front() const {
+if (TVMFFISeqCell::size == 0) {
+ TVM_FFI_THROW(IndexError) << "front() on empty sequence";
+}
+return static_cast(data)[0];
+ }
+
+ /*! \return The last element */
+ const Any& back() const {
+if (TVMFFISeqCell::size == 0) {
+ TVM_FFI_THROW(IndexError) << "back() on empty sequence";
+}
+return static_cast(data)[TVMFFISeqCell::size - 1];
+ }
+
+ /*! \return begin constant iterator */
+ const Any* begin() const { return static_cast(data); }
+
+ /*! \return end constant iterator */
+ const Any* end() const { return begin() + TVMFFISeqCell::size; }
+
+ /*! \brief Release reference to all the elements */
+ void clear() {
+Any* itr = MutableEnd();
+while (TVMFFISeqCell::size > 0) {
+ (--itr)->Any::~Any();
+ --TVMFFISeqCell::size;
+}
+ }
+
+ /*!
+ * \brief Set i-th element of the sequence in-place
+ * \param i The index
+ * \param item The value to be set
+ */
+ void SetItem(int64_t i, Any item) {
+if (i < 0 || i >= TVMFFISeqCell::size) {
+ TVM_FFI_THROW(IndexError) << "Index " << i << " out of bounds " <<
TVMFFISeqCell::size;
+}
+static_cast(data)[i] = std::move(item);
+ }
+
+ /*! \brief Remove the last element */
+ void pop_back() {
+if (TVMFFISeqCell::size == 0) {
+ TVM_FFI_THROW(IndexError) << "pop_back on empty sequence";
+}
+ShrinkBy(1);
+ }
+
+ /*!
+ * \brief Erase element at position idx
+ * \param idx The index to erase
+ */
+ void erase(int64_t idx) {
+if (idx < 0 || idx >= TVMFFISeqCell::size) {
+ TVM_FFI_THROW(IndexError) << "Index " << idx << " out of bounds " <<
TVMFFISeqCell::size;
+}
+MoveElementsLeft(idx, idx + 1, TVMFFISeqCell::size);
+ShrinkBy(1);
+ }
+
+ /*!
+ * \brief Erase elements in half-open range [first, last)
+ * \param first Start index (inclusive)
+ * \param last End index (exclusive)
+ */
+ void erase(int64_t first, int64_t last) {
+if (first == last) return;
+if (first < 0 || last > TVMFFISeqCell::size || first >= last) {
+ TVM_FFI_THROW(IndexError) << "Erase
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2805241301
##
src/ffi/extra/structural_equal.cc:
##
@@ -302,6 +309,33 @@ class StructEqualHandler {
// NOLINTNEXTLINE(performance-unnecessary-value-param)
bool CompareArray(ffi::Array lhs, ffi::Array rhs) {
+return CompareSequence(std::move(lhs), std::move(rhs));
+ }
+
+ // NOLINTNEXTLINE(performance-unnecessary-value-param)
+ bool CompareList(ffi::List lhs, ffi::List rhs) {
+return CompareSequence(std::move(lhs), std::move(rhs));
+ }
+
+ template
+ // NOLINTNEXTLINE(performance-unnecessary-value-param)
+ bool CompareSequence(SeqType lhs, SeqType rhs) {
+const Object* lhs_ptr = lhs.get();
+const Object* rhs_ptr = rhs.get();
+auto pair = std::make_pair(lhs_ptr, rhs_ptr);
+if (active_sequence_pairs_.count(pair)) {
Review Comment:
Still a minimal cycle detection should be okay just in case there's
recursion 🤔
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2805237021
##
include/tvm/ffi/container/list.h:
##
@@ -0,0 +1,523 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ffi/container/list.h
+ * \brief Mutable list type.
+ *
+ * tvm::ffi::List is an erased mutable sequence container.
+ */
+#ifndef TVM_FFI_CONTAINER_LIST_H_
+#define TVM_FFI_CONTAINER_LIST_H_
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace tvm {
+namespace ffi {
+
+/*! \brief List node content in list */
+class ListObj : public SeqBaseObj {
+ public:
+ /*!
+ * \brief Constructs a container with n elements. Each element is a copy of
val
+ * \param n The size of the container
+ * \param val The init value
+ * \return Ref-counted ListObj requested
+ */
+ static ObjectPtr CreateRepeated(int64_t n, const Any& val) {
+ObjectPtr p = ListObj::Empty(n);
+Any* itr = p->MutableBegin();
+for (int64_t& i = p->TVMFFISeqCell::size = 0; i < n; ++i) {
+ new (itr++) Any(val);
+}
+return p;
+ }
+
+ /// \cond Doxygen_Suppress
+ static constexpr const int32_t _type_index = TypeIndex::kTVMFFIList;
+ static const constexpr bool _type_final = true;
+ TVM_FFI_DECLARE_OBJECT_INFO_STATIC(StaticTypeKey::kTVMFFIList, ListObj,
Object);
+ /// \endcond
+
+ private:
+ /*!
+ * \brief Ensure the list has at least n slots.
+ * \param n The lower bound of required capacity.
+ */
+ void Reserve(int64_t n) {
+if (n <= TVMFFISeqCell::capacity) {
+ return;
+}
+Any* old_data = MutableBegin();
+Any* new_data = static_cast(::operator new(sizeof(Any) *
static_cast(n)));
+for (int64_t i = 0; i < TVMFFISeqCell::size; ++i) {
Review Comment:
Wait - just want to double check - is `Any`'s move constructor leak-safe? I
think they are, right?
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
tqchen commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2804205384
##
include/tvm/ffi/container/seq_base.h:
##
@@ -0,0 +1,357 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ffi/container/seq_base.h
+ * \brief Base class for sequence containers (Array, List).
+ */
+#ifndef TVM_FFI_CONTAINER_SEQ_BASE_H_
+#define TVM_FFI_CONTAINER_SEQ_BASE_H_
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+namespace tvm {
+namespace ffi {
+
+/*!
+ * \brief Base class for sequence containers (ArrayObj, ListObj).
+ *
+ * SeqBaseObj is transparent to the FFI type system (no type index),
+ * following the same pattern as BytesObjBase.
+ */
+class SeqBaseObj : public Object, protected TVMFFISeqCell {
+ public:
+ SeqBaseObj() {
+data = nullptr;
+TVMFFISeqCell::size = 0;
+TVMFFISeqCell::capacity = 0;
+data_deleter = nullptr;
+ }
+
+ ~SeqBaseObj() {
+Any* begin = MutableBegin();
+for (int64_t i = 0; i < TVMFFISeqCell::size; ++i) {
+ (begin + i)->Any::~Any();
+}
+if (data_deleter != nullptr) {
+ data_deleter(data);
+}
+ }
+
+ /*! \return The size of the sequence */
+ size_t size() const { return static_cast(TVMFFISeqCell::size); }
+
+ /*! \return The capacity of the sequence */
+ size_t capacity() const { return
static_cast(TVMFFISeqCell::capacity); }
+
+ /*! \return Whether the sequence is empty */
+ bool empty() const { return TVMFFISeqCell::size == 0; }
+
+ /*!
+ * \brief Read i-th element from the sequence.
+ * \param i The index
+ * \return the i-th element.
+ */
+ const Any& at(int64_t i) const { return this->operator[](i); }
+
+ /*!
+ * \brief Read i-th element from the sequence.
+ * \param i The index
+ * \return the i-th element.
+ */
+ const Any& operator[](int64_t i) const {
+if (i < 0 || i >= TVMFFISeqCell::size) {
+ TVM_FFI_THROW(IndexError) << "Index " << i << " out of bounds " <<
TVMFFISeqCell::size;
+}
+return static_cast(data)[i];
+ }
+
+ /*! \return The first element */
+ const Any& front() const {
+if (TVMFFISeqCell::size == 0) {
+ TVM_FFI_THROW(IndexError) << "front() on empty sequence";
+}
+return static_cast(data)[0];
+ }
+
+ /*! \return The last element */
+ const Any& back() const {
+if (TVMFFISeqCell::size == 0) {
+ TVM_FFI_THROW(IndexError) << "back() on empty sequence";
+}
+return static_cast(data)[TVMFFISeqCell::size - 1];
+ }
+
+ /*! \return begin constant iterator */
+ const Any* begin() const { return static_cast(data); }
+
+ /*! \return end constant iterator */
+ const Any* end() const { return begin() + TVMFFISeqCell::size; }
+
+ /*! \brief Release reference to all the elements */
+ void clear() {
+Any* itr = MutableEnd();
+while (TVMFFISeqCell::size > 0) {
+ (--itr)->Any::~Any();
+ --TVMFFISeqCell::size;
+}
+ }
+
+ /*!
+ * \brief Set i-th element of the sequence in-place
+ * \param i The index
+ * \param item The value to be set
+ */
+ void SetItem(int64_t i, Any item) {
+if (i < 0 || i >= TVMFFISeqCell::size) {
+ TVM_FFI_THROW(IndexError) << "Index " << i << " out of bounds " <<
TVMFFISeqCell::size;
+}
+static_cast(data)[i] = std::move(item);
+ }
+
+ /*! \brief Remove the last element */
+ void pop_back() {
+if (TVMFFISeqCell::size == 0) {
+ TVM_FFI_THROW(IndexError) << "pop_back on empty sequence";
+}
+ShrinkBy(1);
+ }
+
+ /*!
+ * \brief Erase element at position idx
+ * \param idx The index to erase
+ */
+ void erase(int64_t idx) {
+if (idx < 0 || idx >= TVMFFISeqCell::size) {
+ TVM_FFI_THROW(IndexError) << "Index " << idx << " out of bounds " <<
TVMFFISeqCell::size;
+}
+MoveElementsLeft(idx, idx + 1, TVMFFISeqCell::size);
+ShrinkBy(1);
+ }
+
+ /*!
+ * \brief Erase elements in half-open range [first, last)
+ * \param first Start index (inclusive)
+ * \param last End index (exclusive)
+ */
+ void erase(int64_t first, int64_t last) {
+if (first == last) return;
+if (first < 0 || last > TVMFFISeqCell::size || first >= last) {
+ TVM_FFI_THROW(IndexError) << "Erase ran
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443: URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2801836082 ## src/ffi/extra/structural_equal.cc: ## @@ -31,6 +32,8 @@ #include #include +#include Review Comment: TODO: we dont allow cycles -- 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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2801823192
##
include/tvm/ffi/container/list.h:
##
@@ -0,0 +1,810 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ffi/container/list.h
+ * \brief Mutable list type.
+ *
+ * tvm::ffi::List is an erased mutable sequence container.
+ */
+#ifndef TVM_FFI_CONTAINER_LIST_H_
+#define TVM_FFI_CONTAINER_LIST_H_
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace tvm {
+namespace ffi {
+
+/*! \brief List node content in list */
+class ListObj : public Object, protected TVMFFISeqCell {
+ public:
+ ~ListObj() {
+Any* begin = MutableBegin();
+for (int64_t i = 0; i < length; ++i) {
+ (begin + i)->Any::~Any();
+}
+if (data_deleter != nullptr) {
+ data_deleter(data);
+}
+ }
+
+ /*! \return The size of the list */
+ size_t size() const { return static_cast(length); }
+
+ /*!
+ * \brief Read i-th element from list.
+ * \param i The index
+ * \return the i-th element.
+ */
+ const Any& at(int64_t i) const { return this->operator[](i); }
+
+ /*!
+ * \brief Read i-th element from list.
+ * \param i The index
+ * \return the i-th element.
+ */
+ const Any& operator[](int64_t i) const {
+if (i < 0 || i >= length) {
+ TVM_FFI_THROW(IndexError) << "Index " << i << " out of bounds " <<
length;
+}
+return MutableBegin()[i];
+ }
+
+ /*! \return begin constant iterator */
+ const Any* begin() const { return MutableBegin(); }
+
+ /*! \return end constant iterator */
+ const Any* end() const { return MutableBegin() + length; }
+
+ /*! \brief Release reference to all the elements */
+ void clear() { ShrinkBy(length); }
+
+ /*! \brief Reverse the elements in-place */
+ void Reverse() { std::reverse(MutableBegin(), MutableBegin() + length); }
+
+ /*!
+ * \brief Set i-th element of the list in-place
+ * \param i The index
+ * \param item The value to be set
+ */
+ void SetItem(int64_t i, Any item) {
+if (i < 0 || i >= length) {
+ TVM_FFI_THROW(IndexError) << "Index " << i << " out of bounds " <<
length;
+}
+MutableBegin()[i] = std::move(item);
+ }
+
+ /*!
+ * \brief Constructs a container with n elements. Each element is a copy of
val
+ * \param n The size of the container
+ * \param val The init value
+ * \return Ref-counted ListObj requested
+ */
+ static ObjectPtr CreateRepeated(int64_t n, const Any& val) {
+ObjectPtr p = ListObj::Empty(n);
+Any* itr = p->MutableBegin();
+for (int64_t& i = p->length = 0; i < n; ++i) {
+ new (itr++) Any(val);
+}
+return p;
+ }
+
+ /// \cond Doxygen_Suppress
+ static constexpr const int32_t _type_index = TypeIndex::kTVMFFIList;
+ static const constexpr bool _type_final = true;
+ TVM_FFI_DECLARE_OBJECT_INFO_STATIC(StaticTypeKey::kTVMFFIList, ListObj,
Object);
+ /// \endcond
+
+ private:
+ /*! \return begin mutable iterator */
+ Any* MutableBegin() const { return static_cast(this->data); }
+
+ /*! \return end mutable iterator */
+ Any* MutableEnd() const { return MutableBegin() + length; }
+
+ /*!
+ * \brief Emplace a new element at the given index
+ * \param idx The index of the element.
+ * \param args The arguments to construct the new element
+ */
+ template
+ void EmplaceInit(size_t idx, Args&&... args) {
+Any* itr = MutableBegin() + idx;
+new (itr) Any(std::forward(args)...);
+ }
+
+ /*!
+ * \brief Assign elements into existing slots from [first, last).
+ * \param idx The starting point
+ * \param first Begin of iterator
+ * \param last End of iterator
+ * \tparam IterType The type of iterator
+ * \note Callers must ensure target slots are already initialized.
+ * \return Self
+ */
+ template
+ ListObj* AssignRange(int64_t idx, IterType first, IterType last) {
+Any* itr = MutableBegin() + idx;
+for (; first != last; ++first) {
+ *itr++ = Any(*first);
+}
+return this;
+ }
+
+ /*!
+ * \brief Move elements from right to left, requires src_begin > dst
+ * \param dst Destination
+ * \param src_begin The s
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2801805291
##
include/tvm/ffi/c_api.h:
##
@@ -356,7 +359,35 @@ typedef struct {
/*! \brief The size of the data. */
size_t size;
} TVMFFIShapeCell;
-// [TVMFFIByteArray.end]
+
+// [TVMFFISeqCell.begin]
+/*!
+ * \brief Sequence cell used by sequence-like containers.
+ *
+ * ArrayObj and ListObj both inherit from this cell.
+ */
+#ifdef __cplusplus
+struct TVMFFISeqCell {
+#else
+typedef struct {
+#endif
+ /*! \brief Data pointer to the first element of the sequence. */
+ void* data;
+ /*! \brief Number of elements used. */
+ int64_t length;
+ /*! \brief Number of elements allocated. */
+ int64_t capacity;
+ /*!
+ * \brief Optional data deleter when data is allocated separately
+ *and its deletion is not managed by object deleter.
+ */
+ void (*data_deleter)(void*);
Review Comment:
clarify what if it's nullptr
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2801806939
##
include/tvm/ffi/container/array.h:
##
@@ -42,20 +42,22 @@ namespace tvm {
namespace ffi {
/*! \brief Array node content in array */
-class ArrayObj : public Object, public details::InplaceArrayBase {
+class ArrayObj : public Object,
Review Comment:
unify `ArrayObj` and `ListObj` on non-resizing operations
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2801804655
##
include/tvm/ffi/c_api.h:
##
@@ -356,7 +359,35 @@ typedef struct {
/*! \brief The size of the data. */
size_t size;
} TVMFFIShapeCell;
-// [TVMFFIByteArray.end]
+
+// [TVMFFISeqCell.begin]
+/*!
+ * \brief Sequence cell used by sequence-like containers.
+ *
+ * ArrayObj and ListObj both inherit from this cell.
+ */
+#ifdef __cplusplus
+struct TVMFFISeqCell {
+#else
+typedef struct {
+#endif
+ /*! \brief Data pointer to the first element of the sequence. */
+ void* data;
+ /*! \brief Number of elements used. */
+ int64_t length;
Review Comment:
rename to `size`
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
tqchen commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r280121
##
src/ffi/extra/serialization.cc:
##
@@ -245,8 +280,20 @@ class ObjectGraphDeserializer {
if (decoded_nodes_[node_index] != nullptr) {
return decoded_nodes_[node_index];
}
+if (active_decode_nodes_.find(node_index) != active_decode_nodes_.end()) {
+ TVM_FFI_THROW(RuntimeError) << "Cyclic references are only supported for
ffi.List in "
+ << "FromJSONGraph";
+}
+active_decode_nodes_.insert(node_index);
Review Comment:
use RAII guard
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
tqchen commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2801775596
##
src/ffi/extra/serialization.cc:
##
@@ -58,92 +62,121 @@ class ObjectGraphSerializer {
if (it != node_index_map_.end()) {
return (*it).second;
}
+const bool supports_cycle = value.type_index() == TypeIndex::kTVMFFIList;
+int64_t node_index = -1;
+if (supports_cycle) {
+ // Pre-register List nodes so self-references can resolve to a stable
index.
+ node_index = static_cast(nodes_.size());
+ node_index_map_.emplace(value, node_index);
+ nodes_.push_back(json::Object{});
+}
+if (active_nodes_.find(value) != active_nodes_.end()) {
+ TVM_FFI_THROW(RuntimeError) << "Cyclic references are not supported in
ToJSONGraph";
+}
+active_nodes_.insert(value);
json::Object node;
-switch (value.type_index()) {
- case TypeIndex::kTVMFFINone: {
-node.Set("type", ffi::StaticTypeKey::kTVMFFINone);
-break;
- }
- case TypeIndex::kTVMFFIBool: {
-node.Set("type", ffi::StaticTypeKey::kTVMFFIBool);
-node.Set("data",
details::AnyUnsafe::CopyFromAnyViewAfterCheck(value));
-break;
- }
- case TypeIndex::kTVMFFIInt: {
-node.Set("type", ffi::StaticTypeKey::kTVMFFIInt);
-node.Set("data",
details::AnyUnsafe::CopyFromAnyViewAfterCheck(value));
-break;
- }
- case TypeIndex::kTVMFFIFloat: {
-node.Set("type", ffi::StaticTypeKey::kTVMFFIFloat);
-node.Set("data",
details::AnyUnsafe::CopyFromAnyViewAfterCheck(value));
-break;
- }
- case TypeIndex::kTVMFFIDataType: {
-DLDataType dtype =
details::AnyUnsafe::CopyFromAnyViewAfterCheck(value);
-node.Set("type", ffi::StaticTypeKey::kTVMFFIDataType);
-node.Set("data", DLDataTypeToString(dtype));
-break;
- }
- case TypeIndex::kTVMFFIDevice: {
-DLDevice device =
details::AnyUnsafe::CopyFromAnyViewAfterCheck(value);
-node.Set("type", ffi::StaticTypeKey::kTVMFFIDevice);
-node.Set("data", json::Array{
- static_cast(device.device_type),
- static_cast(device.device_id),
- });
-break;
- }
- case TypeIndex::kTVMFFISmallStr:
- case TypeIndex::kTVMFFIStr: {
-String str =
details::AnyUnsafe::CopyFromAnyViewAfterCheck(value);
-node.Set("type", ffi::StaticTypeKey::kTVMFFIStr);
-node.Set("data", str);
-break;
- }
- case TypeIndex::kTVMFFISmallBytes:
- case TypeIndex::kTVMFFIBytes: {
-Bytes bytes =
details::AnyUnsafe::CopyFromAnyViewAfterCheck(value);
-node.Set("type", ffi::StaticTypeKey::kTVMFFIBytes);
-node.Set("data", Base64Encode(bytes));
-break;
- }
- case TypeIndex::kTVMFFIArray: {
-Array array =
details::AnyUnsafe::CopyFromAnyViewAfterCheck>(value);
-node.Set("type", ffi::StaticTypeKey::kTVMFFIArray);
-node.Set("data", CreateArrayData(array));
-break;
- }
- case TypeIndex::kTVMFFIMap: {
-Map map =
details::AnyUnsafe::CopyFromAnyViewAfterCheck>(value);
-node.Set("type", ffi::StaticTypeKey::kTVMFFIMap);
-node.Set("data", CreateMapData(map));
-break;
- }
- case TypeIndex::kTVMFFIShape: {
-ffi::Shape shape =
details::AnyUnsafe::CopyFromAnyViewAfterCheck(value);
-node.Set("type", ffi::StaticTypeKey::kTVMFFIShape);
-node.Set("data", Array(shape->data, shape->data +
shape->size));
-break;
- }
- default: {
-if (value.type_index() >= TypeIndex::kTVMFFIStaticObjectBegin) {
- // serialize type key since type index is runtime dependent
- node.Set("type", value.GetTypeKey());
- node.Set("data", CreateObjectData(value));
-} else {
- TVM_FFI_THROW(RuntimeError) << "Cannot serialize type `" <<
value.GetTypeKey() << "`";
- TVM_FFI_UNREACHABLE();
+try {
Review Comment:
use RAII scope instead of try catch here
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
gemini-code-assist[bot] commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2800377362
##
python/tvm_ffi/container.py:
##
@@ -209,6 +224,146 @@ def __radd__(self, other: Iterable[T]) -> Array[T]:
return type(self)(itertools.chain(other, self))
+@register_object("ffi.List")
+class List(core.Object, MutableSequence[T]):
+"""Mutable list container that represents a mutable sequence in the FFI."""
+
+# tvm-ffi-stubgen(begin): object/ffi.List
+# fmt: off
+# fmt: on
+# tvm-ffi-stubgen(end)
+
+def __init__(self, input_list: Iterable[T] = ()) -> None:
+"""Construct a List from a Python sequence."""
+self.__init_handle_by_constructor__(_ffi_api.List, *input_list)
+
+@overload
+def __getitem__(self, idx: SupportsIndex, /) -> T: ...
+
+@overload
+def __getitem__(self, idx: slice, /) -> list[T]: ...
+
+def __getitem__(self, idx: SupportsIndex | slice, /) -> T | list[T]: #
ty: ignore[invalid-method-override]
+"""Return one element or a list for a slice."""
+length = len(self)
+return getitem_helper(self, _ffi_api.ListGetItem, length, idx)
+
+@overload
+def __setitem__(self, index: SupportsIndex, value: T) -> None: ...
+
+@overload
+def __setitem__(self, index: slice[int | None], value: Iterable[T]) ->
None: ...
+
+def __setitem__(self, index: SupportsIndex | slice[int | None], value: T |
Iterable[T]) -> None:
+"""Set one element or assign a slice."""
+if isinstance(index, slice):
+replacement = list(cast(Iterable[T], value))
+length = len(self)
+start, stop, step = index.indices(length)
+if step != 1:
+target_indices = list(range(start, stop, step))
+if len(replacement) != len(target_indices):
+raise ValueError(
+"attempt to assign sequence of size "
+f"{len(replacement)} to extended slice of size
{len(target_indices)}"
+)
+for i, item in zip(target_indices, replacement):
+_ffi_api.ListSetItem(self, i, item)
+return
+stop = max(stop, start)
+_ffi_api.ListReplaceSlice(self, start, stop,
type(self)(replacement))
+return
+
+normalized_index = normalize_index(len(self), index)
+_ffi_api.ListSetItem(self, normalized_index, cast(T, value))
+
+@overload
+def __delitem__(self, index: SupportsIndex) -> None: ...
+
+@overload
+def __delitem__(self, index: slice[int | None]) -> None: ...
+
+def __delitem__(self, index: SupportsIndex | slice[int | None]) -> None:
+"""Delete one element or a slice."""
+if isinstance(index, slice):
+length = len(self)
+start, stop, step = index.indices(length)
+if step == 1:
+stop = max(stop, start)
+_ffi_api.ListEraseRange(self, start, stop)
+else:
+indices = (
+reversed(range(start, stop, step)) if step > 0 else
range(start, stop, step)
+)
+for i in indices:
+_ffi_api.ListErase(self, i)
+return
+normalized_index = normalize_index(len(self), index)
+_ffi_api.ListErase(self, normalized_index)
+
+def insert(self, index: int, value: T) -> None:
+"""Insert value before index."""
+length = len(self)
+if index < 0:
+index = max(0, index + length)
+else:
+index = min(index, length)
+_ffi_api.ListInsert(self, index, value)
+
+def append(self, value: T) -> None:
+"""Append one value to the tail."""
+_ffi_api.ListAppend(self, value)
+
+def clear(self) -> None:
+"""Remove all elements from the list."""
+_ffi_api.ListClear(self)
+
+def pop(self, index: int = -1) -> T:
+"""Remove and return item at index (default last)."""
+length = len(self)
+if length == 0:
+raise IndexError("pop from empty list")
+normalized_index = normalize_index(length, index)
+return cast(T, _ffi_api.ListPop(self, normalized_index))
+
+def extend(self, values: Iterable[T]) -> None:
+"""Append elements from an iterable."""
+for value in values:
+_ffi_api.ListAppend(self, value)
Review Comment:
The current implementation of `extend` calls `ListAppend` in a loop. This
can be inefficient as it may lead to multiple reallocations of the underlying
buffer if the list needs to grow. A more efficient approach is to use slice
assignment, which will perform at most one reallocation for the entire
extension.
```suggestion
def extend(self, values: Iterable[T]) -> None:
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on PR #443: URL: https://github.com/apache/tvm-ffi/pull/443#issuecomment-3892542137 /gemini review -- 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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
gemini-code-assist[bot] commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2797530530
##
python/tvm_ffi/container.py:
##
@@ -124,6 +126,19 @@ def getitem_helper(
return elem_getter(obj, index)
+def normalize_index(length: int, idx: SupportsIndex) -> int:
+"""Normalize and bounds-check a Python index."""
+try:
+index = operator.index(idx)
+except TypeError as exc: # pragma: no cover - defensive, matches list
behaviour
+raise TypeError(f"indices must be integers or slices, not
{type(idx).__name__}") from exc
+if index < -length or index >= length:
+raise IndexError(f"Index out of range. size: {length}, got index
{index}")
+if index < 0:
+index += length
+return index
Review Comment:
This new function `normalize_index` duplicates logic for index normalization
and bounds checking from the existing `getitem_helper` function. To improve
maintainability and avoid code duplication, consider refactoring
`getitem_helper` to use this new `normalize_index` function in a future change.
This would likely involve moving `normalize_index` before `getitem_helper` in
the file.
##
include/tvm/ffi/container/list.h:
##
@@ -0,0 +1,767 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ffi/container/list.h
+ * \brief Mutable list type.
+ *
+ * tvm::ffi::List is an erased mutable sequence container.
+ */
+#ifndef TVM_FFI_CONTAINER_LIST_H_
+#define TVM_FFI_CONTAINER_LIST_H_
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace tvm {
+namespace ffi {
+
+/*! \brief List node content in list */
+class ListObj : public Object, protected TVMFFISeqCell {
+ public:
+ ~ListObj() {
+Any* begin = MutableBegin();
+for (int64_t i = 0; i < length; ++i) {
+ (begin + i)->Any::~Any();
+}
+TVM_FFI_ICHECK(data_deleter != nullptr);
+data_deleter(data);
+ }
+
+ /*! \return The size of the list */
+ size_t size() const { return static_cast(length); }
+
+ /*!
+ * \brief Read i-th element from list.
+ * \param i The index
+ * \return the i-th element.
+ */
+ const Any& at(int64_t i) const { return this->operator[](i); }
+
+ /*!
+ * \brief Read i-th element from list.
+ * \param i The index
+ * \return the i-th element.
+ */
+ const Any& operator[](int64_t i) const {
+if (i < 0 || i >= length) {
+ TVM_FFI_THROW(IndexError) << "Index " << i << " out of bounds " <<
length;
+}
+return MutableBegin()[i];
+ }
+
+ /*! \return begin constant iterator */
+ const Any* begin() const { return MutableBegin(); }
+
+ /*! \return end constant iterator */
+ const Any* end() const { return MutableBegin() + length; }
+
+ /*! \brief Release reference to all the elements */
+ void clear() { ShrinkBy(length); }
+
+ /*!
+ * \brief Set i-th element of the list in-place
+ * \param i The index
+ * \param item The value to be set
+ */
+ void SetItem(int64_t i, Any item) {
+if (i < 0 || i >= length) {
+ TVM_FFI_THROW(IndexError) << "Index " << i << " out of bounds " <<
length;
+}
+MutableBegin()[i] = std::move(item);
+ }
+
+ /*!
+ * \brief Constructs a container with n elements. Each element is a copy of
val
+ * \param n The size of the container
+ * \param val The init value
+ * \return Ref-counted ListObj requested
+ */
+ static ObjectPtr CreateRepeated(int64_t n, const Any& val) {
+ObjectPtr p = ListObj::Empty(n);
+Any* itr = p->MutableBegin();
+for (int64_t& i = p->length = 0; i < n; ++i) {
+ new (itr++) Any(val);
+}
+return p;
+ }
+
+ /// \cond Doxygen_Suppress
+ static constexpr const int32_t _type_index = TypeIndex::kTVMFFIList;
+ static const constexpr bool _type_final = true;
+ TVM_FFI_DECLARE_OBJECT_INFO_STATIC(StaticTypeKey::kTVMFFIList, ListObj,
Object);
+ /// \endcond
+
+ private:
+ /*! \return begin mutable iterator */
+ Any* MutableBegin() const { return static_cast(this->data); }
+
+ /*! \return end mutable iterator */
+
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on PR #443: URL: https://github.com/apache/tvm-ffi/pull/443#issuecomment-3889491004 /gemini review -- 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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
gemini-code-assist[bot] commented on code in PR #443:
URL: https://github.com/apache/tvm-ffi/pull/443#discussion_r2797349289
##
src/ffi/extra/serialization.cc:
##
@@ -312,6 +331,15 @@ class ObjectGraphDeserializer {
return array;
}
+ List DecodeListData(const json::Array& data) {
+List list;
+list.reserve(static_cast(data.size()));
+for (const auto& elem : data) {
+ list.push_back(GetOrDecodeNode(elem.cast()));
+}
+return list;
+ }
Review Comment:
This function is very similar to `DecodeArrayData`. To avoid code
duplication, you could use a template function.
For example:
```cpp
template
SeqType DecodeSequenceData(const json::Array& data) {
SeqType seq;
seq.reserve(static_cast(data.size()));
for (const auto& elem : data) {
seq.push_back(GetOrDecodeNode(elem.cast()));
}
return seq;
}
```
Then `DecodeArrayData` could be `return
DecodeSequenceData>(data);` and this function could be `return
DecodeSequenceData>(data);`.
##
src/ffi/extra/serialization.cc:
##
@@ -152,6 +159,15 @@ class ObjectGraphSerializer {
return data;
}
+ json::Array CreateArrayData(const List& value) {
+json::Array data;
+data.reserve(static_cast(value.size()));
+for (const Any& item : value) {
+ data.push_back(GetOrCreateNodeIndex(item));
+}
+return data;
+ }
Review Comment:
This function is identical to `CreateArrayData(const Array&)`. To avoid
code duplication, you could use a template function that can handle both
`Array` and `List`.
For example:
```cpp
template
json::Array CreateSequenceData(const SeqType& value) {
json::Array data;
data.reserve(static_cast(value.size()));
for (const Any& item : value) {
data.push_back(GetOrCreateNodeIndex(item));
}
return data;
}
```
Then both `CreateArrayData` overloads could call this template.
--
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]
Re: [PR] feat: Introduce List as Mutable Sequence [tvm-ffi]
junrushao commented on PR #443: URL: https://github.com/apache/tvm-ffi/pull/443#issuecomment-3889328294 /gemini review -- 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]
