-
Notifications
You must be signed in to change notification settings - Fork 24
feat: Update asio to 1.34 #31
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.
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
feat: Update asio to 1.34 #31
Conversation
|
@FlorianReimold That should be everything. Would be good to get this and the CMake policy update merged and a new version tagged. I'm working on some packaging stuff currently (for the AUR) and need this repo to be asio 1.34 compatible. 😄 |
FlorianReimold
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DownerCase: Thanks for taking care of this! I only stumbled across the asio::get_associated_allocator calls. Why do we need those? The returned allocator is not used.
| me->synchronous_callback_(data_buffer, header); | ||
| }); | ||
| }, | ||
| asio::get_associated_allocator(me->data_strand_)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this line of code necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the remaining signatures of io_context_strand::post requires an allocator even though the function doesn't use it...
Looking at the commit that removed the originally used function, it actually says to use asio::post which does not require an allocator.
chriskohlhoff/asio@2f9c4ef#diff-a9951ef828e91a6ea4683ee5f1da1046abaefaa093db694a8a9825a098f50431
Thank you. I will make that change.
| me->readHeaderLength(); | ||
| }); | ||
| }, | ||
| asio::get_associated_allocator(me->data_strand_)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this line of code necessary?
| me->synchronous_callback_ = callback; | ||
| }); | ||
| }, | ||
| asio::get_associated_allocator(data_strand_)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this line of code necessary?
a82d76e to
7c03080
Compare
|
Ok @FlorianReimold all sorted again. Seems like asio was confusing the 2015 toolchain, didn't dig deep into why but the 2017 toolchain doesn't fail with the same code 🤷 |
|
VS 2017 toolchain is fine. Asio probably now uses some C++17 features. |
Updates asio to 1.34 to resolve build failures when trying to use new versions.
I don't know why I had to increase the test timeout but I can reproduce the failure on master with the asio 1.32. 🤷
Nb: There is still use of deprecated functionality with the data strand.wrapfunction but it doesn't cause the build to fail..wrapforasio::bind_executorand set the compile definition to disallow using deprecated functionality 😄Closes #30