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

Fix ASan report about alloc/dealloc mismatch when compiled with C++23 #633

Merged
merged 8 commits into from
Jun 25, 2024

Conversation

waahm7
Copy link
Contributor

@waahm7 waahm7 commented Jun 24, 2024

Issue #, if available:

Description of changes:
Duplicate of #629 to fix lint and submodules

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

Choose a reason for hiding this comment

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

If std::allocator<T> doesn't have any virtual functions ... does that mean nothing should inherit from it?

Copy link
Contributor Author

@waahm7 waahm7 Jun 24, 2024

Choose a reason for hiding this comment

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

Discussed offline, we will backlog this for now. We should probably be using https://docs.ros.org/en/foxy/Tutorials/Advanced/Allocator-Template-Tutorial.html

@@ -51,6 +51,10 @@ namespace Aws
return static_cast<RawPointer>(aws_mem_acquire(m_allocator, n * sizeof(T)));
}

#if _LIBCPP_STD_VER > 20
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really need #if here?
does it really matter if we expose it in all versions of cpp standard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

std::allocation_result was added in _LIBCPP_STD_VER > 20

@waahm7 waahm7 requested a review from DmitriyMusatkin June 25, 2024 20:31
@waahm7 waahm7 merged commit 2be4b09 into main Jun 25, 2024
59 checks passed
@waahm7 waahm7 deleted the cpp23-warning-fix branch June 25, 2024 20:40
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.

4 participants