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

<flat_map>: Various cleanups and bug fixes #4468

Merged

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Mar 10, 2024

Towards #2910. Addresses review comments in #4337 and #4386.

Style cleanups:

  • Eliminate redundant parentheses.
  • Use it[-1] instead of *(it - 1).
  • Add some const. Not yet to iterators due to moving.
  • Eliminate most if {/*...*/ return; } else {/*...*/}, except when returning end()/cend() in else.

Bug fixes:

  • Use direct-non-list-initialization.
  • Avoid calling distance on non-forward input iterators.
  • Fix iterator dereferencing in insertion.

Drive-by consistency changes:

  • Make the default (template) arguments of deduction guides relying on allocator<byte> conditionally present, which is consistent with polymorphic_allocator. The behavior should be unchanged (modulo the existence of byte) when __cpp_lib_byte is not defined.

Performance improvements:

  • Short-circuit evaluation of _Hint_is_accurate.
  • Eliminate _Key_equal after lower_bound.
  • Unwrap iterators before calling algorithms (lower_bound, upper_bound, and lexicographical_compare_three_way).

Test changes:

  • Inspect the return value of erase_if.
  • Show different effects of insertion for different containers.
  • Extract conditional_t.
  • Test SCARY-ness for various containers (including deque).

- Eliminate redundant parentheses.
- Use `it[-1]` instead of `*(it - 1)`.
- Add some `const`. Not yet to iterators due to moving.
- Eliminate most `if {/*...*/ return; } else {/*...*/}`, except when returning `end()`/`cend()` in `else`.
Also fix iterator dereferencing in insertion
- `lower_bound`
- `upper_bound`
- `lexicographical_compare_three_way`
- Inspect the return value of `erase_if`
- Show different effects of insertion for different containers
- Extract `conditional_t`
- Test SCARY-ness for various containers (including `deque`)
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner March 10, 2024 17:51
@StephanTLavavej StephanTLavavej added bug Something isn't working flat_meow C++23 container adaptors labels Mar 10, 2024
@StephanTLavavej StephanTLavavej self-assigned this Mar 10, 2024
stl/inc/flat_map Show resolved Hide resolved
stl/inc/flat_map Show resolved Hide resolved
stl/inc/flat_map Show resolved Hide resolved
@StephanTLavavej StephanTLavavej merged commit a3bc9c7 into microsoft:feature/flat_map Mar 13, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks, this is great! I went ahead and merged this, with a few comments for followup. 😻 🥞 🗺️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flat_meow C++23 container adaptors
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants