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

rbtree::emplace_hint() compilation fix for variadic arg use #392

Merged
merged 2 commits into from
Oct 15, 2020

Conversation

miherius
Copy link
Contributor

This PR provides a fix for the rbtree::emplace_hint() method, when it called using multiply variadic template arguments. The problem is in DoInsertValueHint() method, which can't accept the forwarded parameter pack, expecting an already constructed pair only. A small example:

eastl::map<int, char> m;
m.emplace(1, 'a'); // ok, arguments are forwarded to the `pair` constructor
m.emplace_hint(m.end(), 2, 'b'); // compilation fail, arguments can't be forwarded
m.emplace_hint(m.end(), eastl::make_pair(2, 'b')); // ok, passing a constructed `pair`

This PR makes the interface of the DoInsertValueHint() method (internal helper for "hint" versions of insert methods) similar to the DoInsertValue() method (another helper for non-"hint" insert methods), so it can be used with variadic template arguments now.

Additional fixes:

  • rbtree::try_emplace() move rvalue-ref key instead of copy;
  • rbtree::try_emplace() delegates value_type construction to the internal logic;
  • "hint"-version of non-unique hashtable::insert() can be complied now (was needed for new tests).

Map and multimap tests have been extended to cover more use-cases of these methods (changes in TestMapCpp11(), TestMultimapCpp11() functions).

There are no changes in the public interface of the mentioned classes.

@rparolin rparolin changed the title brtree::emplace_hint() compilation fix for variadic arg use rbtree::emplace_hint() compilation fix for variadic arg use Oct 15, 2020
@rparolin
Copy link
Contributor

rparolin commented Oct 15, 2020

Thanks for the PR! Looks good to me.

@rparolin rparolin merged commit dcff00d into electronicarts:master Oct 15, 2020
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.

None yet

2 participants