Handle non-trivially-copyable types in Loop/Scan output concatenation

[yuslepukhin]

This pull request improves the handling of string tensors in the CPU implementation of the Loop operator, ensuring correct memory management and copy semantics for non-trivially-copyable types like std::string. It also adds comprehensive unit tests to verify these behaviors, especially for cases involving string scan outputs and loop-carried variables.

Enhancements for string tensor support:

• Updated ConcatenateCpuOutput in loop.cc to properly detect string tensors and use std::copy for concatenation, ensuring correct handling of heap-allocated string payloads and avoiding unsafe byte-wise copying. [1] [2]
• Modified OutputIterator::ZeroOutCurrent in scan_utils.h to skip zeroing for string tensors, as their elements are already default-constructed and cannot be safely set with memset.

New and extended unit tests for string tensor scenarios:

• Added tests to loop_test.cc covering:
  • String scan outputs, including long strings that exceed the small-string-optimization threshold and multi-element outputs.
  • Loop-carried string variables to ensure they are copied correctly.
  • Zero-iteration cases to confirm empty string scan outputs are handled without errors.

[Copilot]

Pull request overview

This PR fixes CPU Loop scan-output concatenation for non-trivially-copyable element types (notably std::string) by avoiding byte-wise copying/zeroing that can corrupt string objects, and adds unit tests to validate correct behavior for string scan outputs and loop-carried variables.

Changes:

• Update Loop CPU scan-output concatenation to use std::copy for tensor(string) outputs instead of byte-wise copying.
• Update Scan OutputIterator::ZeroOutCurrent to skip zeroing for string tensors (since memset is unsafe for std::string).
• Add Loop CPU provider tests covering string scan outputs (single/multi-element), loop-carried strings, and zero-iteration behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
onnxruntime/core/providers/cpu/controlflow/loop.cc	Use proper element-wise copy semantics when concatenating string scan outputs.
onnxruntime/core/providers/cpu/controlflow/scan_utils.h	Avoid memset-style zeroing for string tensors in Scan output iteration.
onnxruntime/test/providers/cpu/controlflow/loop_test.cc	Add unit tests validating Loop behavior with string scan outputs and loop-carried strings (including zero-iteration cases).

[tianleiwu]

Review Summary

The change correctly fixes a real correctness bug: Loop/Scan output concatenation previously byte-copied tensor payloads unconditionally, which is undefined behavior for non-trivially-copyable element types like std::string (it would copy raw pointers/SSO bytes and lead to double-frees or corruption). Routing string tensors through std::copy over DataAsSpan<std::string>() and skipping the memset-based ZeroOutCurrent for strings are both correct.

Strengths

• Correct handling of heap-allocated string payloads (SSO-exceeding strings are explicitly exercised in tests).
• output_span is now constructed once outside the loop for the non-string path, which also resolves the earlier reviewer note about per-iteration span re-creation.
• The ZeroOutCurrent skip is safe because string tensor buffers are placement-new default-constructed to empty strings; the comment documents this clearly.
• Strong test coverage: single/multi-element string scan outputs, loop-carried string variable, and the zero-iteration empty-output edge case.

Non-blocking suggestions (see inline)

• Consider std::move instead of std::copy for the per-iteration string payloads, since per_iteration_output is discarded after concatenation.

Overall this looks good to merge. Only minor, optional nits below.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.