Skip to content

fix(firestore): add noexcept specifiers to move, swap, hash operations - #16117

Merged
dlarocque merged 4 commits into
mainfrom
dl/noexcept
May 4, 2026
Merged

fix(firestore): add noexcept specifiers to move, swap, hash operations#16117
dlarocque merged 4 commits into
mainfrom
dl/noexcept

Conversation

@dlarocque

@dlarocque dlarocque commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

b/269108721

Adds noexcept specifiers to move, swap, and hash operations throughout the SDK.

According the C++ Core Guidelines, move, swap, and hash operations should be noexcept.
If move operations are throwable, many std lib containers that use these objects will fall back to using copy semantics, degrading performance.

I used grep to attempt to locate all instances of these operations that did not have the noexcept specifiers.

Command:

rg --pcre2 -n -g '*.{h,cc}' '\b(?:friend\s+)?(?:void|size_t|std::size_t|auto)\s+(?:swap|hash|operator\s*\(\))\s*\(' | rg -v 'noexcept'

Notes:

  • Default move constructors are nothrow. I added noexcept anyway to be more explicit.
  • Some of the hash operations are throwable. For example, some hash functions call functions like absl::StrCat which can throw std::bad_alloc if memory can't be allocated (OOM). If we label these hash operations as noexcept, and then a std::bad_alloc is thrown, then std::terminate will be called [1]. This is ok, since OOM conditions are already fatal and cause apps to crash.

[1] Whenever an exception is thrown and the search for a handler encounters the outermost block of a non-throwing function, the function std::terminate is called https://en.cppreference.com/cpp/language/noexcept_spec

b/269108721

Adds `noexcept` specifiers to move, swap, and hash operations throughout
the SDK.

According the C++ Core Guidelines, move, swap, and hash operations
should be `noexcept`.
If move operations are throwable, many std lib containers that use these
objects will fall back to using copy semantics, degrading performance.

I used grep to attempt to locate all instances of these operations
that did not have the `noexcept` specifiers.

Command:
```
rg --pcre2 -n -g '*.{h,cc}' '\b(?:friend\s+)?(?:void|size_t|std::size_t|auto)\s+(?:swap|hash|operator\s*\(\))\s*\(' | rg -v 'noexcept'
```

Results:

Deleted constructors. No action required.
- util/statusor_internals.h:248:  TraitsBase(TraitsBase&&) = delete;
- util/statusor_internals.h:250:  TraitsBase& operator=(TraitsBase&&) = delete;
- util/ordered_code.h:155:  OrderedCode(OrderedCode&&) = delete;
- util/ordered_code.h:156:  OrderedCode& operator=(OrderedCode&&) = delete;
- remote/datastore.h:154:  Datastore(Datastore&& other) = delete;
- remote/datastore.h:156:  Datastore& operator=(Datastore&& other) = delete;
- util/testing_hooks.h:155:  TestingHooks(TestingHooks&&) = delete;
- util/testing_hooks.h:157:  TestingHooks& operator=(TestingHooks&&) = delete;
- local/memory_document_overlay_cache.h:39:  MemoryDocumentOverlayCache(MemoryDocumentOverlayCache&&) = delete;
- local/memory_document_overlay_cache.h:40:  MemoryDocumentOverlayCache& operator=(MemoryDocumentOverlayCache&&) = delete;
- local/leveldb_document_overlay_cache.h:51:  LevelDbDocumentOverlayCache(LevelDbDocumentOverlayCache&&) = delete;
- local/leveldb_document_overlay_cache.h:52:  LevelDbDocumentOverlayCache& operator=(LevelDbDocumentOverlayCache&&) = delete;

Default constructors. Are implicitly noexcept. Explicitly make noexcept.
[x] remote/bloom_filter.h:35:  BloomFilter(BloomFilter&&) = default;
[x] remote/bloom_filter.h:37:  BloomFilter& operator=(BloomFilter&&) = default;
[x] util/statusor_internals.h:230:  TraitsBase(TraitsBase&&) = default;
[x] util/statusor_internals.h:232:  TraitsBase& operator=(TraitsBase&&) = default;
[x] util/statusor_internals.h:239:  TraitsBase(TraitsBase&&) = default;
[x] util/statusor_internals.h:241:  TraitsBase& operator=(TraitsBase&&) = default;
[x] util/random_access_queue.h:59:  RandomAccessQueue(RandomAccessQueue&& other) = default;
[x] util/random_access_queue.h:68:  RandomAccessQueue& operator=(RandomAccessQueue&& other) = default;
[x] api/settings.h:52:  Settings(Settings&& other) = default;
[x] api/settings.h:55:  Settings& operator=(Settings&& other) = default;

Utils copied, this is not code written by us. No action required.
[ ] util/status.h:140:      void operator()(const State* ptr) const;
[ ] util/statusor.h:166:  StatusOr& operator=(Status&& status);
[ ] util/statusor.h:243:StatusOr<T>& StatusOr<T>::operator=(Status&& status) {
[ ] util/statusor.h:116:  StatusOr(StatusOr&&) = default;
[ ] util/statusor.h:117:  StatusOr& operator=(StatusOr&&) = default;

[x] model/document_key.h:113:  size_t operator()(const DocumentKey& key) const;
[x] model/overlay.h:79:  std::size_t operator()(const Overlay&) const;
   - could be an issue if Mutation is not nothrow hashable. It hashes type, key, mutation, precondition. I think these should all be nothrow hashable.
[ ] model/field_path.cc:84:  void operator()(T* out, const std::string& segment) {
   - i don't think this can be made noexcept, since it's templated. T-> could throw an exception.
[x] core/query.h:332:  size_t operator()(const firebase::firestore::core::Query& query) const {
   - Query::Has() -> absl::StrCat() -> std::badalloc in OOM. Already fatal. std::terminate ok. use noexcept.
[x] core/pipeline_util.h:204:  size_t operator()(
   - Query::Has() -> absl::StrCat() -> std::badalloc in OOM. Already fatal. std::terminate ok. use noexcept.
[x] core/pipeline_util.h:212:  size_t operator()(
   - Query::Has() -> absl::StrCat() -> std::badalloc in OOM. Already fatal. std::terminate ok. use noexcept.
[x] core/target.h:245:  size_t operator()(const firebase::firestore::core::Target& target) const {
@gemini-code-assist

Copy link
Copy Markdown
Contributor
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

@dlarocque
dlarocque marked this pull request as ready for review April 24, 2026 15:01
@dlarocque
dlarocque requested a review from cherylEnkidu April 24, 2026 16:08

@cherylEnkidu cherylEnkidu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank u for this improvement!

@dlarocque
dlarocque merged commit 7ba8411 into main May 4, 2026
74 checks passed
@dlarocque
dlarocque deleted the dl/noexcept branch May 4, 2026 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants