Skip to content

Conversation

armallen
Copy link
Contributor

@armallen armallen commented Oct 13, 2025

threadpool has c++17/c++20 compatibility issues because result_of is removed with c++20, and was deprecated in c++17. invoke_result is used instead.

Standard is updated to c++17 in albatross, which involves fixing albatross::apply calls.

@armallen armallen requested a review from peddie October 13, 2025 16:43
Copy link
Contributor

@peddie peddie left a comment

Choose a reason for hiding this comment

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

Is this C++20 or LLVM 20?

@armallen
Copy link
Contributor Author

Is this C++20 or LLVM 20?

@peddie llvm20 issues a deprecation warning when building with c++17, and we have "warning as errors" config.
Then c++20 removes it altogether, so it also fails.

I see albatross uses c++14 standard, should I update CMake/Bazel build to c++17?

@armallen armallen changed the title threadpool c++20 compatibility update threadpool c++17/c++20 compatibility update Oct 14, 2025
branches:
- 'master'

concurrency:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a commit to a PR will stop running workflows

};

const auto proposed_params = apply(values, get_params);
const auto proposed_params = albatross::apply(values, get_params);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was trying to use std::apply, which has different arguments

@armallen armallen changed the title threadpool c++17/c++20 compatibility update Update standard to c++17 Oct 14, 2025
Copy link
Contributor

@peddie peddie left a comment

Choose a reason for hiding this comment

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

Seems good to me.

@armallen armallen changed the title Update standard to c++17 Update standard to c++17 [OI-3403] Oct 15, 2025
@armallen armallen merged commit fb70445 into master Oct 15, 2025
9 checks passed
@armallen armallen deleted the armallen/tpup branch October 15, 2025 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants