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

[FixedMap/EnumMap] Fix emplace to work with non-copyable/non-moveable types and piecewise construction #85

Conversation

alexkaratarakis
Copy link
Collaborator

@alexkaratarakis alexkaratarakis commented Dec 28, 2023

No description provided.

@@ -499,8 +499,22 @@ class EnumMapBase
template <class... Args>
constexpr std::pair<iterator, bool> emplace(Args&&... args) noexcept
{
std::pair<K, V> as_pair{std::forward<Args>(args)...};

Choose a reason for hiding this comment

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

im curious, how does std::pair<K, V> as_pair{std::forward<Args>(args)...}; work if the parameter pack has sizeof...(Args) != 2 values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This raises a good point. Since std::map stores std::pair, the emplace target is a pair.
Hence, the original code was creating a std::pair and then using that, so possible compilation issues would be handled by constructing the std::pair.

This would reject parameter packs that were not compatible (including sizeof...(Args) != 2 although you can actually have different count). However, this is problematic for non-moveable/non-copyable types due to the temporary pair.

I have revised the code and tests to be more explicit about what is supported.

@alexkaratarakis alexkaratarakis force-pushed the emplace_map_non_moveable_non_copyable branch from 37378eb to ed5cb9b Compare December 28, 2023 07:23
@alexkaratarakis alexkaratarakis changed the title [FixedMap/EnumMap] Fix emplace to work with non-copyable/non-moveable… [FixedMap/EnumMap] Fix emplace to work with non-copyable/non-moveable types and piecewise construction Dec 28, 2023
@alexkaratarakis alexkaratarakis merged commit ed5cb9b into teslamotors:main Jan 3, 2024
5 checks passed
@alexkaratarakis alexkaratarakis deleted the emplace_map_non_moveable_non_copyable branch January 3, 2024 02:38
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