Skip to content

fix(find_first_of): add missing early return and fix datapar iterator dependency#7161

Merged
hkaiser merged 2 commits intoTheHPXProject:masterfrom
aneek22112007-tech:fix/find-first-of-missing-early-exit
Apr 13, 2026
Merged

fix(find_first_of): add missing early return and fix datapar iterator dependency#7161
hkaiser merged 2 commits intoTheHPXProject:masterfrom
aneek22112007-tech:fix/find-first-of-missing-early-exit

Conversation

@aneek22112007-tech
Copy link
Copy Markdown
Contributor

@aneek22112007-tech aneek22112007-tech commented Apr 9, 2026

Summary

Fixes a correctness/consistency bug in parallel find_first_of and a compilation error in its SIMD implementation.

1. The Core Bug (Parallel Partition)

In libs/core/algorithms/include/hpx/parallel/algorithms/detail/find.hpp, the inner search loop over needles did not short-circuit.

// FIXED
for (FwdIter2 iter = s_first; iter != s_last; ++iter)
{
    if (HPX_INVOKE(cmp, v, *iter))
    {
        tok.cancel(i);
        return; // Added this missing return
    }
}

2. SIMD (datapar) Fix

Fixed a bug in libs/core/algorithms/include/hpx/parallel/datapar/find.hpp where the code used it + idx, which would fail to compile for non-random-access iterators (like std::forward_list). Now correctly uses the value and index provided by the loop utility.

3. Coding Standards (inspect)

  • Added missing #include <atomic> in findfirstof_tests.hpp.
  • Replaced non-ASCII characters () in comments with standard ASCII (->) to satisfy the inspect tool.

Testing

Updated find_first_of_edge_cases_test() to verify:

  • Empty ranges, single-element ranges, and predicate invocation count consistency (regression guard for the short-circuit).
  • All tests passed locally with inspect clean.

@StellarBot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Apr 9, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

TIP This summary will be updated as you push new changes. Give us feedback

hkaiser
hkaiser previously approved these changes Apr 10, 2026
Copy link
Copy Markdown
Contributor

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

Nice catch! LGTM!

@hkaiser hkaiser added this to the 2.0.0 milestone Apr 10, 2026
@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Apr 11, 2026

Could you please fix the inspect issues reported. Also, please rebase onto master and resolve the conflicts.

@aneek22112007-tech aneek22112007-tech force-pushed the fix/find-first-of-missing-early-exit branch from 8090700 to b7d2826 Compare April 11, 2026 19:15
@aneek22112007-tech
Copy link
Copy Markdown
Contributor Author

Hi @hkaiser,

I have rebased the PR onto the latest master and resolved the conflicts.

I also fixed the inspect issues by adding the missing header and replacing the non-ASCII characters in the test comments with standard ones. I've verified the changes locally by running the inspect tool and the find_first_of unit tests, and everything is now passing.

Thanks for the review!

@aneek22112007-tech
Copy link
Copy Markdown
Contributor Author

It appears GitHub automatically closed this PR because, after rebasing and applying the requested inspect fixes, the branch became identical to master (as the changes were already integrated via #7163).

Since the fix and the coding standard improvements are now live in the repository, this PR is indeed redundant. Thank you for the guidance!

@aneek22112007-tech aneek22112007-tech changed the title fix(find_first_of): add missing early return in parallel partition inner loop fix(find_first_of): add missing early return and fix datapar iterator dependency Apr 11, 2026
Signed-off-by: Aneek22112007 <das.aneek007@gmail.com>
@aneek22112007-tech aneek22112007-tech force-pushed the fix/find-first-of-missing-early-exit branch from 3a4fd6c to 346ad43 Compare April 11, 2026 19:38
Copy link
Copy Markdown
Contributor

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@hkaiser hkaiser merged commit 8ccf948 into TheHPXProject:master Apr 13, 2026
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants