Skip to content

Conversation

@mmichel11
Copy link
Contributor

__red_by_seg_op has a potential ambiguous return when the return type of __binary_op differs from the input segment's underlying value type. To resolve this issue, we can ensure the second element in the tuple returned by __red_by_seg_op matches the return type of __binary_op through a conversion.

In practice, this issue was encountered when performing a reduce_by_segment operation over zip iterators where a std::tuple is explicitly returned by the reduction operator. I have added a test case which catches this issue in reduce_by_segment_zip.pass

Signed-off-by: Matthew Michel <matthew.michel@intel.com>
Signed-off-by: Matthew Michel <matthew.michel@intel.com>
Signed-off-by: Matthew Michel <matthew.michel@intel.com>
@mmichel11 mmichel11 added this to the 2022.9.0 milestone Apr 25, 2025
@mmichel11 mmichel11 marked this pull request as ready for review April 26, 2025 22:43
danhoeflinger
danhoeflinger previously approved these changes Apr 30, 2025
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Matthew Michel <matthew.michel@intel.com>
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@adamfidel adamfidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now!

@mmichel11 mmichel11 merged commit 81f22d3 into main May 1, 2025
19 checks passed
@mmichel11 mmichel11 deleted the dev/mmichel11/red_by_seg_tup_fix branch May 1, 2025 14:11
timmiesmith pushed a commit that referenced this pull request Jun 9, 2025
Signed-off-by: Matthew Michel <matthew.michel@intel.com>
danhoeflinger pushed a commit that referenced this pull request Oct 29, 2025
Signed-off-by: Matthew Michel <matthew.michel@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants