Skip to content

futurecpp: avoid overlapping ranges in zip_iterator mutation tests - #658

Merged
4og merged 2 commits into
eclipse-score:mainfrom
4og:fix/zip-iterator-test-overlapping-swap
Oct 7, 2026
Merged

4og merged 2 commits into
eclipse-score:mainfrom
4og:fix/zip-iterator-test-overlapping-swap

Conversation

@4og

@4og 4og commented Oct 5, 2026

Copy link
Copy Markdown
Member

Problem

Two tests in zip_iterator_test.cpp fail on QNX x86_64 but pass on Linux x86_64 and QNX aarch64:

  • container_test_basics.swap (all three container types)
  • container_test_bi_directional.use_in_stl_algorithm_with_bidirectional_iterator_requirement (both bidirectional container types)

Both tests zipped two offset views of the same container {a, b, c} = {1.2, 3.4, 5.6}.
The two zipped positions therefore referred to (a, b) and (b, c), sharing the storage of b.
Both tests then swapped dereferenced zip iterators, swap(*iter1, *iter2) directly and std::reverse via iter_swap.

Root cause

zip_iterator::operator* returns a zipped_tuple_like holding references to the underlying elements.
Its ADL swap delegates to std::swap on the underlying std::tuple, which swaps the tuple elements pairwise: swap(a, b) and swap(b, c).
Since both modify b, the result depends on the order of these two swaps:

Element swap order (a, b, c) afterwards
swap(a, b), then swap(b, c) (3.4, 5.6, 1.2)
swap(b, c), then swap(a, b) (5.6, 1.2, 3.4)

libstdc++ swaps tuple elements left to right.
libc++ (used by QNX) implements tuple::swap as std::__swallow(__tuple_leaf<I, T>::swap(...)...), i.e. the element swaps are function arguments whose evaluation order is unspecified in C++17.
The QNX GCC aarch64 build evaluates them left to right, the x86_64 build right to left, which yields 5.6 instead of the expected 3.4.

More fundamentally, (a, b) and (b, c) cannot be exchanged as independent records: after a true exchange, b would have to hold both the original a and the original c.
The tests relied on unspecified behavior rather than on a property of zip_iterator.

Fix

The two mutating tests now zip two independent containers ({1.2, 3.4} and {5.6, 7.8}), so every zipped position refers to distinct storage and the swap result is independent of element swap order.
Coverage is kept or improved:

  • swap still checks both swapping iterator objects and swapping the dereferenced zipped values, and now asserts both positions after each step.
  • The bidirectional test still runs std::reverse and now asserts both positions afterwards.

The zip_iterator implementation is unchanged.
Read-only tests (find, post_increment, copy_assignment, etc.) keep using offset views of a single container, since overlap is harmless there.

Alternatives considered

  • Force left-to-right element swaps in zipped_tuple_like's swap. This would make results identical across platforms, but the outcome for overlapping ranges would still not be a meaningful exchange. It would change library behavior only to satisfy an invalid test assumption.
  • Disable the tests on QNX x86_64 or weaken assertions. Loses coverage without addressing the order-dependent setup.

@4og
4og temporarily deployed to workflow-approval October 5, 2026 09:51 — with GitHub Actions Inactive
@4og
4og temporarily deployed to workflow-approval October 5, 2026 09:51 — with GitHub Actions Inactive
@4og
4og temporarily deployed to workflow-approval October 5, 2026 09:51 — with GitHub Actions Inactive
@4og
4og temporarily deployed to workflow-approval October 5, 2026 09:51 — with GitHub Actions Inactive
@github-actions github-actions Bot added c++ C++ code comp-futurecpp Related to score/language/futurecpp component labels Oct 5, 2026
@4og
4og marked this pull request as draft October 5, 2026 09:56
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Documentation preview for this pull request is available at:
pr-658: https://eclipse-score.github.io/baselibs/pr-658/

@4og
4og force-pushed the fix/zip-iterator-test-overlapping-swap branch from 6119804 to 664b43a Compare October 5, 2026 10:09
@4og
4og temporarily deployed to workflow-approval October 5, 2026 10:09 — with GitHub Actions Inactive
@4og
4og temporarily deployed to workflow-approval October 5, 2026 10:09 — with GitHub Actions Inactive
@4og
4og temporarily deployed to workflow-approval October 5, 2026 10:09 — with GitHub Actions Inactive
@4og
4og temporarily deployed to workflow-approval October 5, 2026 10:09 — with GitHub Actions Inactive

@Ehrja Ehrja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The change looks fine to me. To my knowledge there is no known usecase for using the same container in overlapping manner within zip-iterator within BMW scope.

From the analysis it really originates in unspecified execution-order and thus relies on underlying "system-libraries".
I just wonder whether it is possible to "emit" the semantics of "this quirk" to users in a better way than just encoded in existing unit-tests. So maybe a doxy-note or something in public-api 🤔

4og and others added 2 commits October 6, 2026 12:39
@4og
4og force-pushed the fix/zip-iterator-test-overlapping-swap branch from 664b43a to b720a2e Compare October 6, 2026 10:41
@4og
4og temporarily deployed to workflow-approval October 6, 2026 10:41 — with GitHub Actions Inactive
@4og
4og temporarily deployed to workflow-approval October 6, 2026 10:41 — with GitHub Actions Inactive
@4og
4og temporarily deployed to workflow-approval October 6, 2026 10:41 — with GitHub Actions Inactive
@4og
4og temporarily deployed to workflow-approval October 6, 2026 10:41 — with GitHub Actions Inactive
@github-actions github-actions Bot added the ci CI related changes label Oct 6, 2026
@4og 4og self-assigned this Oct 6, 2026
@4og
4og marked this pull request as ready for review October 6, 2026 10:43
@4og
4og merged commit 2890b97 into eclipse-score:main Oct 7, 2026
27 of 28 checks passed

This branch was previously deployed

1 inactive deployment
workflow-approval — b720a2eb Deployed Oct 6, 2026 by 4og via Build and Test aarch64-qnx (examples/integration) / approval #2256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ C++ code ci CI related changes comp-futurecpp Related to score/language/futurecpp component

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants