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.

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

session manager in memory, addressing TODO. #3817

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

devnexen
Copy link
Collaborator

does not seem to bring down performance.

@coveralls
Copy link

coveralls commented Nov 30, 2023

Coverage Status

coverage: 92.062% (+0.007%) from 92.055%
when pulling 862504c on memory_manager_copy_if
into 7bc2b1c on master.

Copy link
Collaborator

@reneme reneme left a comment

Choose a reason for hiding this comment

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

Given that iOS and Android were updated a while ago, perhaps we might try and get our feet wet with ranges here. Also that might actually improve readability.

@devnexen
Copy link
Collaborator Author

devnexen commented Dec 7, 2023

ok giving a try..

does not seem to bring down performance.
@devnexen
Copy link
Collaborator Author

devnexen commented Dec 7, 2023

should we update the fuzzer's toolchain too ?

@reneme
Copy link
Collaborator

reneme commented Dec 7, 2023

Thanks for trying this! Could you elaborate a bit on "the issue with clang < 16", please? To me it looks like the ranges implementation simply wasn't ready for prime time, yet.

If that's true, I feel, we should just avoid range operations altogether, unfortunately. We committed to supporting Clang 14 for Botan 3.x), if it's not ready for (some) ranges features, it's just better to not use it at all, in my opinion. 😭

should we update the fuzzer's toolchain too ?

@randombit do we even have a way of doing that in OSS-Fuzz?

@devnexen
Copy link
Collaborator Author

devnexen commented Dec 7, 2023

If that's true, I feel, we should just avoid range operations altogether, unfortunately. We committed to supporting Clang 14 for Botan 3.x), if it's not ready for (some) ranges features, it's just better to not use it at all, in my opinion. 😭

Yes if you choose clang 14 as minimum I m afraid we can't indeed, clang is behind gcc, on average, in term of C++20 support.

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.

3 participants