Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[execution] Add schedule_result_t alias template. #5846

Merged
merged 2 commits into from
May 26, 2022
Merged

[execution] Add schedule_result_t alias template. #5846

merged 2 commits into from
May 26, 2022

Conversation

SAtacker
Copy link
Contributor

@SAtacker SAtacker commented Apr 12, 2022 •
edited by hkaiser
Loading

  • Alias schedule_result_t gives the result type of schedule

Signed-off-by: Shreyas Atre shreyasatre16@gmail.com

Any background context you want to provide?

  • Implement P0443/P1897/P2300 #5045 (comment)
  • 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 the random numbers generated are valid inputs for the tests.

@jenkins-cscs
Copy link
Collaborator

Can one of the admins verify this patch?

@SAtacker SAtacker marked this pull request as ready for review April 12, 2022 18:42
@hkaiser hkaiser added this to the 1.9.0 milestone Apr 13, 2022
@hkaiser
Copy link
Member

hkaiser commented Apr 13, 2022

add to whitelist

@hkaiser hkaiser mentioned this pull request Apr 13, 2022
Open
hkaiser
hkaiser previously approved these changes Apr 13, 2022
View reviewed changes
Copy link
Member

@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! I would like to wait with merging this until V1.8 has been released (should not take long now).

@gonidelis
Copy link
Contributor

Nice!

@SAtacker
[execution] Add schedule_result_t alias template.
66b5348 
- Alias schedule_result_t gives result type of schedule

Signed-off-by: Shreyas Atre <shreyasatre16@gmail.com>
@SAtacker
[execution] remove requires for schedule_result_t
62479f3 
- As per http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/
p2300r4.html#spec-execution.schedulers Point number 2
> 2. Let S be the type of a scheduler ...
S is already a scheduler, no need to check

Suggested-by: Agustín Bergé <agustinberge@gmail.com>
Signed-off-by: Shreyas Atre <shreyasatre16@gmail.com>
hkaiser
hkaiser approved these changes May 26, 2022
View reviewed changes
Copy link
Member

@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.

The Windows builder issue was fixed on master, I guess we can ignore it here. LGTM, thanks!

@hkaiser
Copy link
Member

hkaiser commented May 26, 2022

bors merge

@bors
Copy link

bors bot commented May 26, 2022

Build succeeded:

@bors bors bot merged commit 25248a1 into STEllAR-GROUP:master May 26, 2022
@hkaiser hkaiser modified the milestones: 1.9.0, 1.8.1 Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hkaiser hkaiser hkaiser approved these changes

@aurianer aurianer Awaiting requested review from aurianer

@biddisco biddisco Awaiting requested review from biddisco

@msimberg msimberg Awaiting requested review from msimberg

Assignees
No one assigned
Labels
category: executors category: senders/receivers Implementations of the p0443r14 / p2300 + p1897 proposals type: compatibility issue type: enhancement
Projects
None yet
Milestone
1.8.1
Development

Successfully merging this pull request may close these issues.
4 participants
@SAtacker @jenkins-cscs @hkaiser @gonidelis