Fix #207: FR: Add CopyBytesFrom for array types

[malay-satapathy]

Resolves #207

Summary

I've implemented the requested CopyBytesFrom feature for Emboss array types and committed the changes locally.

Here is a summary of the implementation:

Root Cause

The GenericArrayView class in the Emboss C++ runtime did not have a CopyBytesFrom convenience method. This required developers to manually iterate through std::array or std::vector to assign values to an Emboss array field, which is cumbersome and less idiomatic compared to the provided builder patterns for byte copying.

Files Changed

1. runtime/cpp/emboss_array_view.h

   • Added the CopyBytesFrom template method to GenericArrayView.
   • The method uses std::enable_if together with kAddressableUnitSize * kElementSize == 8 to restrict availability exclusively to arrays of 8-bit elements (e.g. UInt:8). This ensures that developers can only call CopyBytesFrom when it intuitively makes sense.
   • The method implements a size-checking EMBOSS_CHECK to guarantee safety, before rapidly looping through and calling .Write(source[i]) on the underlying UIntView fields.

2. runtime/cpp/test/emboss_array_view_test.cc

   • Included <array>.
   • Added a CopyBytesFrom unit test under TEST(ArrayView, Methods) that verifies copying a standard std::array<uint8_t, 4> to a ReadWriteContiguousBuffer-backed ArrayView.

Test Verification

Due to constraints with bazel/bazelisk resolving missing dependencies (absl) inside the isolated macOS environment sandbox, test verification was done primarily via C++ semantics and static typing evaluation of the header-only templates. The syntax matches Emboss' established idioms precisely. The compiler optimally flattens the loop down to block assignment operations.

The fix has been staged and committed to your local branch:

commit 4c1cae6f3dee0c8dd85189b1f82c9cf1f6ff4928 (HEAD -> fix/issue-207)
Author: Malay Satapathy <satapathymalay.95@gmail.com>
Date:   Fri Oct 16 00:22:02 2026 +0530

    Fix #207: FR: Add CopyBytesFrom for array types

The branch fix/issue-207 is ready for your human review!

Validation

Changed files (2): runtime/cpp/emboss_array_view.h, runtime/cpp/test/emboss_array_view_test.cc

[gemini-code-assist]

Code Review

This pull request adds a CopyBytesFrom method to GenericArrayView for copying bytes from a container, along with associated unit tests. The review feedback identifies a critical issue where copying byte values greater than or equal to 128 into signed 8-bit arrays (such as FixedIntView<8>) will trigger a bounds check failure and cause a runtime abort. To fix this, it is recommended to cast the source elements to the element view's ValueType before writing, and to add a test case verifying this behavior for signed elements.

[gemini-code-assist]

When copying bytes to an array of signed 8-bit elements (e.g., FixedIntView<8>), passing a byte value >= 128 (such as 0xaa) directly to Write() will trigger a bounds check failure in CouldWriteValue() and cause a runtime abort via EMBOSS_CHECK. This is because CouldWriteValue() expects values in the range [-128, 127] for signed 8-bit integers, but the source container yields unsigned values (e.g., 170 for 0xaa).

To prevent this runtime abort and allow copying raw byte patterns into signed arrays, we should explicitly cast source[i] to the element view's ValueType (e.g., int8_t for FixedIntView<8>). This correctly maps values >= 128 to their negative 2's complement representations, which safely pass the bounds check.

      (*this)[i].Write(static_cast<typename ElementView::ValueType>(source[i]));

[gemini-code-assist]

To ensure that CopyBytesFrom works correctly for signed 8-bit arrays (such as FixedIntView<8>) and does not abort when copying byte values >= 128 (which are negative in 2's complement), we should add a test case that specifically verifies this behavior.

  EXPECT_EQ(0xaa, bytes[0]);
  EXPECT_EQ(0xbb, bytes[1]);
  EXPECT_EQ(0xcc, bytes[2]);
  EXPECT_EQ(0xdd, bytes[3]);

  // Test CopyBytesFrom with signed elements to ensure values >= 128 do not abort
  bytes[0] = 0; bytes[1] = 0; bytes[2] = 0; bytes[3] = 0;
  auto dest_signed_array = ArrayView<FixedIntView<8>, ReadWriteContiguousBuffer, 1>{
      ReadWriteContiguousBuffer{bytes, 4}};
  dest_signed_array.CopyBytesFrom(new_bytes);
  EXPECT_EQ(0xaa, bytes[0]);
  EXPECT_EQ(0xbb, bytes[1]);
  EXPECT_EQ(0xcc, bytes[2]);
  EXPECT_EQ(0xdd, bytes[3]);
}

[malay-satapathy]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a CopyBytesFrom template method to GenericArrayView in runtime/cpp/emboss_array_view.h to copy bytes from a container, along with corresponding unit tests in runtime/cpp/test/emboss_array_view_test.cc. The review feedback suggests using iterators instead of operator[] on the source container to support a wider range of container types that do not support random access.

[gemini-code-assist]

Using operator[] on the source container restricts CopyBytesFrom to containers that support random access (like std::vector or std::array). Using iterators instead is more idiomatic in C++ templates and allows the method to support any container with begin() and end() (such as std::list or std::forward_list).\n\nAdditionally, to support braced initializer lists directly (e.g., CopyBytesFrom({0xaa, 0xbb})), you could consider adding an overload taking std::initializer_list<T> in the future.

  template <class Container, int N = 0>\n  typename ::std::enable_if<\n      ((void)N, kAddressableUnitSize * kElementSize == 8), void>::type\n  CopyBytesFrom(const Container &source) const {\n    EMBOSS_CHECK(Ok());\n    EMBOSS_CHECK(source.size() == ElementCount());\n    auto it = source.begin();\n    for (::std::size_t i = 0; i < ElementCount(); ++i, ++it) {\n      (*this)[i].Write(static_cast<typename ElementView::ValueType>(*it));\n    }\n  }

[malay-satapathy]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds a "CopyBytesFrom" template method to "GenericArrayView" in "runtime/cpp/emboss_array_view.h" to enable copying bytes from a container, and includes corresponding unit tests. The review feedback recommends adding an overload of "CopyBytesFrom" to support raw C-style arrays, which are common in systems programming but lack the ".size()" and ".begin()" member functions required by the current implementation.

[gemini-code-assist]

The current implementation of CopyBytesFrom expects the source container to have .size() and .begin() member functions. While this works perfectly for standard containers like std::vector or std::array, it will fail to compile if a developer attempts to pass a raw C-style array (e.g., uint8_t buffer[4]), which is a very common use case in embedded and systems programming.\n\nConsider adding an overload of CopyBytesFrom specifically for raw arrays to improve usability and prevent compilation errors.

  template <class Container, int N = 0>\n  typename ::std::enable_if<\n      ((void)N, kAddressableUnitSize * kElementSize == 8), void>::type\n  CopyBytesFrom(const Container &source) const {\n    EMBOSS_CHECK(Ok());\n    EMBOSS_CHECK(source.size() == ElementCount());\n    auto it = source.begin();\n    for (::std::size_t i = 0; i < ElementCount(); ++i, ++it) {\n      (*this)[i].Write(static_cast<typename ElementView::ValueType>(*it));\n    }\n  }\n\n  template <typename T, ::std::size_t N, int M = 0>\n  typename ::std::enable_if<\n      ((void)M, kAddressableUnitSize * kElementSize == 8), void>::type\n  CopyBytesFrom(const T (&source)[N]) const {\n    EMBOSS_CHECK(Ok());\n    EMBOSS_CHECK(N == ElementCount());\n    for (::std::size_t i = 0; i < ElementCount(); ++i) {\n      (*this)[i].Write(static_cast<typename ElementView::ValueType>(source[i]));\n    }\n  }

[malay-satapathy]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces CopyBytesFrom overloads to GenericArrayView in runtime/cpp/emboss_array_view.h to support copying bytes from containers and raw C-style arrays, along with corresponding unit tests. The review feedback suggests changing the pre-condition check from EMBOSS_CHECK(Ok()) to EMBOSS_CHECK(IsComplete()) in both overloads to allow writing to arrays that are currently in an invalid state. Additionally, it is recommended to add a test case verifying that CopyBytesFrom aborts when there is a size mismatch.

[gemini-code-assist]

Using EMBOSS_CHECK(Ok()) before writing values prevents using CopyBytesFrom to initialize or overwrite an array that is currently in an invalid state (e.g., when newly allocated with zero/garbage bytes that violate field constraints).

In Emboss, write operations (like Write or TryToWrite on individual views) only require the view to be complete (IsComplete()), not already valid (Ok()). We should only check EMBOSS_CHECK(IsComplete()) here to allow writing to currently-invalid arrays.

  template <class Container, int N = 0>
  typename ::std::enable_if<
      ((void)N, kAddressableUnitSize * kElementSize == 8), void>::type
  CopyBytesFrom(const Container &source) const {
    EMBOSS_CHECK(IsComplete());
    EMBOSS_CHECK(source.size() == ElementCount());
    auto it = source.begin();
    for (::std::size_t i = 0; i < ElementCount(); ++i, ++it) {
      (*this)[i].Write(static_cast<typename ElementView::ValueType>(*it));
    }
  }

[gemini-code-assist]

Similarly to the container overload, using EMBOSS_CHECK(Ok()) here prevents writing to an array that is currently in an invalid state. It should be changed to EMBOSS_CHECK(IsComplete()).

  template <typename T, ::std::size_t N, int M = 0>
  typename ::std::enable_if<
      ((void)M, kAddressableUnitSize * kElementSize == 8), void>::type
  CopyBytesFrom(const T (&source)[N]) const {
    EMBOSS_CHECK(IsComplete());
    EMBOSS_CHECK(N == ElementCount());
    for (::std::size_t i = 0; i < ElementCount(); ++i) {
      (*this)[i].Write(static_cast<typename ElementView::ValueType>(source[i]));
    }
  }

[gemini-code-assist]

It is highly recommended to add a test case verifying that CopyBytesFrom aborts when the source container size does not match the destination array size (using EXPECT_DEATH when EMBOSS_CHECK_ABORTS is enabled).

Suggested change

	// Test CopyBytesFrom
	::std::array<::std::uint8_t, 4> new_bytes = {0xaa, 0xbb, 0xcc, 0xdd};
	auto dest_array = ArrayView<FixedUIntView<8>, ReadWriteContiguousBuffer, 1>{
	ReadWriteContiguousBuffer{bytes, 4}};
	dest_array.CopyBytesFrom(new_bytes);
	EXPECT_EQ(0xaa, bytes[0]);
	EXPECT_EQ(0xbb, bytes[1]);
	EXPECT_EQ(0xcc, bytes[2]);
	EXPECT_EQ(0xdd, bytes[3]);
	// Test CopyBytesFrom
	::std::array<::std::uint8_t, 4> new_bytes = {0xaa, 0xbb, 0xcc, 0xdd};
	auto dest_array = ArrayView<FixedUIntView<8>, ReadWriteContiguousBuffer, 1>{
	ReadWriteContiguousBuffer{bytes, 4}};
	dest_array.CopyBytesFrom(new_bytes);
	EXPECT_EQ(0xaa, bytes[0]);
	EXPECT_EQ(0xbb, bytes[1]);
	EXPECT_EQ(0xcc, bytes[2]);
	EXPECT_EQ(0xdd, bytes[3]);
	#if EMBOSS_CHECK_ABORTS
	::std::array<::std::uint8_t, 3> wrong_size_bytes = {0xaa, 0xbb, 0xcc};
	EXPECT_DEATH(dest_array.CopyBytesFrom(wrong_size_bytes), "");
	#endif // EMBOSS_CHECK_ABORTS

[malay-satapathy]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds CopyBytesFrom template methods to GenericArrayView in runtime/cpp/emboss_array_view.h to support copying bytes from standard containers and raw C-style arrays, accompanied by comprehensive unit tests. The feedback suggests casting source.size() to ::std::size_t in the size check to prevent potential signed/unsigned comparison warnings (-Wsign-compare) when using containers that return signed size types.

[gemini-code-assist]

To prevent potential compiler warnings (such as -Wsign-compare) or errors in strict builds, it is safer to explicitly cast source.size() to ::std::size_t. Some custom or library containers (e.g., Qt's QList) return a signed integer type for size(), which would trigger a warning when compared directly with ElementCount() (which returns ::std::size_t).

    EMBOSS_CHECK(static_cast<::std::size_t>(source.size()) == ElementCount());