Skip to content

make_future: drop static_assert from detail::make_future#7282

Merged
hkaiser merged 1 commit into
TheHPXProject:masterfrom
guptapratykshh:fix/make_future-revert-7248-static-assert
May 20, 2026
Merged

make_future: drop static_assert from detail::make_future#7282
hkaiser merged 1 commit into
TheHPXProject:masterfrom
guptapratykshh:fix/make_future-revert-7248-static-assert

Conversation

@guptapratykshh
Copy link
Copy Markdown
Contributor

@guptapratykshh guptapratykshh commented May 20, 2026

Proposed Changes

  • Replace the static_assert inside detail::make_future(Sender, Allocator) (introduced by make_future: static_assert against silent-hang run_loop_scheduler sender #7248) with HPX_ASSERT_MSG and keep the sender_completion_is_run_loop_scheduler trait that backs it. The static_assert was a hard error during template substitution and broke tag_priority's overload-resolution probing on GCC 12 debug, surfacing in rostam CDash build 53308 on the algorithm_run_loop.cpp test cases that use make_future(just(3) | continues_on(sched)). HPX_ASSERT_MSG is a no-op in release builds and a runtime check in debug, so it does not interfere with substitution, while still flagging the silent-hang misuse during development.
  • Add #include <hpx/assert.hpp> to make_future.hpp for HPX_ASSERT_MSG.
  • Document the rationale inline: the public make_future CPO's tag_override_invoke overload already routes make_future(run_loop_sender) to the scheduler-form tag_invoke (which builds future_data_with_run_loop and drives loop.run() in execute_deferred), so reaching this 1-arg fallback with a run-loop completion scheduler indicates a direct call to detail::make_future rather than going through the public API; the runtime assert exists to flag that bypass in debug, not to harden release-mode misuse.

Any background context you want to provide?

Checklist

Not all points below apply to all pull requests.

  • I have added a new feature and have added tests to go along with it.
  • I have fixed a bug and have added a regression test.
  • I have added a test using random numbers; I have made sure it uses a seed, and that random numbers generated are valid inputs for the tests.

@guptapratykshh guptapratykshh requested a review from hkaiser as a code owner May 20, 2026 03:28
Copilot AI review requested due to automatic review settings May 20, 2026 03:28
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 20, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@StellarBot
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes a non-SFINAE-friendly static_assert (and its helper trait) from detail::make_future(Sender, Allocator) that was introduced to prevent a known runtime hang for run-loop scheduled senders, and replaces it with an explanatory comment. The PR relies on the public make_future CPO’s existing tag_override_invoke routing to ensure run-loop completion schedulers are handled by the scheduler-form overload that drives loop.run().

Changes:

  • Removed sender_completion_is_run_loop_scheduler and the static_assert in detail::make_future(Sender, Allocator) that could break GCC 12 debug due to template probing/instantiation.
  • Added a comment explaining why the compile-time guard is not present in the fallback and where the actual run-loop driving behavior is implemented.

Comment thread libs/core/execution/include/hpx/execution/algorithms/make_future.hpp Outdated
@guptapratykshh guptapratykshh force-pushed the fix/make_future-revert-7248-static-assert branch from 64bf4c9 to 8097af5 Compare May 20, 2026 03:35
@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented May 20, 2026

@guptapratykshh Could you turn that static_assert into a runtime assert (HPX_ASSERT)? That will not break compilation of the good cases but will still flag the issue at least at runtime.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread libs/core/execution/include/hpx/execution/algorithms/make_future.hpp Outdated
…SERT_MSG to preserve runtime guard without breaking GCC 12 debug SFINAE

Signed-off-by: guptapratykshh <pratykshgupta9999@gmail.com>
@guptapratykshh guptapratykshh force-pushed the fix/make_future-revert-7248-static-assert branch from cef6e84 to 2f0dba0 Compare May 20, 2026 14:44
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 added this to the 2.0.0 milestone May 20, 2026
@hkaiser hkaiser merged commit 59afa33 into TheHPXProject:master May 20, 2026
69 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