CXX-2139 update clang-tidy configuration with WarningsAsErrors

[eramongodb]

Resolves CXX-2139. Followup to #1495 upon realising the clang-tidy EVG task has been providing minimal value due to the absence of WarningsAsErrors checks (most/all diagnostics except for hard compilation errors are emitted but otherwise ignored). #1049 implemented a "temporary" workaround clang-tidy configuration options document this absence of static analysis errors. This PR proposes taking this opportunity to codify the clang-tidy configuration options for Clang 21.1.1 (on rhel9-latest) and the initial set of WarningsAsErrors checks to be enabled going forward.

The set of clang-tidy warnings emitted by the clang-tidy task with currently proposed changes are visible in this patch build.

---

The .clang-tidy configuration file has been updated with the dump of the default configuration for clang-tidy 21.1.1 (the version on rhel9-latest where this task will now be executed). The default list of checks is preserved for now, including the disabling of all default checks with -* (with redundancy removed). The initial changes in this PR leave the set of WarningsAsErrors diagnostics (which unfortunately does not seem to support list syntax) empty: this PR is an opportunity to decide on an initially minimal-and-constrainted set of checks which will fail the clang-tidy task on error.

Important

Adding checks to WarningsAsErrors is necessary to determine when the clang-tidy EVG task fails due to static analysis diagnostics.

The initial set of files to be analyzed are library components only. Notably, the benchmark (requires C++17), config, _deps, docs, enums, test, directories and macro guard headers are excluded from analysis. We can consider extending analysis to one or more of these extra directories once we have decided the set of WarningsAsErrors diagnostics to enable. The current filter list includes 219 source files:

$ find src -type f \( -name *.cc -o -name *.cpp \) | perl -lne 'print if m$.*/(?:bsoncxx|mongocxx)/lib/.*$' | wc -l
219

The --header-filter flag further limits analysis to library headers only so that diagnostics are not emitted for external headers.

Note

These filters and exclusions are deliberately not specified via .clang-tidy configuration options to avoid pessimizing the local development experience. The .clang-tidy configuration only excludes config, _deps and enums headers.

According to llvm/llvm-project#47042 (which is finally resolved by llvm/llvm-project#154012), the "N warnings generated" spam (which is what motivated the 2>/dev/null that masked the "command not found" error) should hopefully be in the next clang-tidy release. 👏 Until then, we'll need to continue using this workaround.

Given this configuration and filters, the current state of clang-tidy warnings is as follows:

• Total warnings emitted: 266.
• Warnings sorted from top to bottom by highest unique occurance count:
  • cppcoreguidelines-pro-type-union-access: 87 warnings
  • cppcoreguidelines-pro-bounds-array-to-pointer-decay: 34 warnings
  • cppcoreguidelines-init-variables: 20 warnings
  • cppcoreguidelines-pro-type-reinterpret-cast: 19 warnings
  • cppcoreguidelines-avoid-magic-numbers: 17 warnings
  • cppcoreguidelines-pro-bounds-pointer-arithmetic: 17 warnings
  • cppcoreguidelines-pro-type-const-cast: 12 warnings
  • cppcoreguidelines-pro-type-member-init: 12 warnings
  • cert-oop54-cpp: 11 warnings
  • cppcoreguidelines-avoid-c-arrays: 9 warnings
  • cppcoreguidelines-rvalue-reference-param-not-moved: 5 warnings
  • cppcoreguidelines-use-enum-class: 5 warnings
  • cppcoreguidelines-use-default-member-init: 4 warnings
  • cppcoreguidelines-owning-memory: 3 warnings
  • cppcoreguidelines-prefer-member-initializer: 3 warnings
  • cppcoreguidelines-avoid-non-const-global-variables: 2 warnings
  • cppcoreguidelines-pro-bounds-constant-array-index: 2 warnings
  • cert-err58-cpp: 1 warning
  • cppcoreguidelines-avoid-do-while: 1 warning
  • cppcoreguidelines-pro-type-static-cast-downcast: 1 warning
  • cppcoreguidelines-pro-type-vararg: 1 warning

Tip

See here for the list of all available checks with clang-tidy 21.1 and their descriptions.

All checks currently enabled by the default configuration are listed below (82 in total; 21 of these currently emit at least one a diagnostic):

cert-arr39-c
cert-con36-c
cert-con54-cpp
cert-ctr56-cpp
cert-dcl03-c
cert-dcl16-c
cert-dcl37-c
cert-dcl50-cpp
cert-dcl51-cpp
cert-dcl54-cpp
cert-dcl58-cpp
cert-dcl59-cpp
cert-env33-c
cert-err09-cpp
cert-err33-c
cert-err34-c
cert-err52-cpp
cert-err58-cpp
cert-err60-cpp
cert-err61-cpp
cert-exp42-c
cert-fio38-c
cert-flp30-c
cert-flp37-c
cert-int09-c
cert-mem57-cpp
cert-msc24-c
cert-msc30-c
cert-msc32-c
cert-msc33-c
cert-msc50-cpp
cert-msc51-cpp
cert-msc54-cpp
cert-oop11-cpp
cert-oop54-cpp
cert-oop57-cpp
cert-oop58-cpp
cert-pos44-c
cert-pos47-c
cert-sig30-c
cert-str34-c
cppcoreguidelines-avoid-c-arrays
cppcoreguidelines-avoid-capturing-lambda-coroutines
cppcoreguidelines-avoid-const-or-ref-data-members
cppcoreguidelines-avoid-do-while
cppcoreguidelines-avoid-goto
cppcoreguidelines-avoid-magic-numbers
cppcoreguidelines-avoid-non-const-global-variables
cppcoreguidelines-avoid-reference-coroutine-parameters
cppcoreguidelines-c-copy-assignment-signature
cppcoreguidelines-explicit-virtual-functions
cppcoreguidelines-init-variables
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-macro-to-enum
cppcoreguidelines-macro-usage
cppcoreguidelines-misleading-capture-default-by-value
cppcoreguidelines-missing-std-forward
cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcoreguidelines-no-suspend-with-lock
cppcoreguidelines-noexcept-destructor
cppcoreguidelines-noexcept-move-operations
cppcoreguidelines-noexcept-swap
cppcoreguidelines-non-private-member-variables-in-classes
cppcoreguidelines-owning-memory
cppcoreguidelines-prefer-member-initializer
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
cppcoreguidelines-pro-type-const-cast
cppcoreguidelines-pro-type-cstyle-cast
cppcoreguidelines-pro-type-member-init
cppcoreguidelines-pro-type-reinterpret-cast
cppcoreguidelines-pro-type-static-cast-downcast
cppcoreguidelines-pro-type-union-access
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-rvalue-reference-param-not-moved
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
cppcoreguidelines-use-default-member-init
cppcoreguidelines-use-enum-class
cppcoreguidelines-virtual-class-destructor

[kevinAlbs]

Opinion: enable WarningsAsErrors for default checks that do not already warn. Enable checks straightforward to address. Defer others to future work:

Some default checks appear to be aliases:

• cert-con54-cpp Omit? Alias of cert-con36-c
• cert-dcl51-cpp Omit? Alias of cert-dcl37-c
• cert-err61-cpp Omit? Alias of cert-err09-cpp
• cert-msc50-cpp Omit? Alias of cert-msc30-c
• cert-msc51-cpp Omit? Alias of cert-msc32-c

Some appear straightforward to address:

• cert-oop54-cpp Enable? 11 warnings. On brief look, self-assignments appear to be correct but doing a needless copy. I expect these can be fixed with an added self-check, from:

  read_concern& read_concern::operator=(read_concern const& other) {
      _impl = bsoncxx::make_unique<impl>(libmongoc::read_concern_copy(other._impl->read_concern_t));
      return *this;
  }

  to:

  read_concern& read_concern::operator=(read_concern const& other) {
      if (this == &other) {
          return *this;
      }
      _impl = bsoncxx::make_unique<impl>(libmongoc::read_concern_copy(other._impl->read_concern_t));
      return *this;
  }

• cppcoreguidelines-init-variables (Enable? 20 warnings. Appears easy to address).
• cppcoreguidelines-prefer-member-initializer (Enable? 3 warnings. Appears easy to address.)
• cppcoreguidelines-pro-type-static-cast-downcast (Enable? 1 warning. May be a legitimate bug?)
• cppcoreguidelines-use-default-member-init (Enable? 4 warnings. Appears easy to fix)

[eramongodb]

Important

The clang-tidy EVG task now fails when a diagnostic enabled by Checks is emitted but not explicitly excluded by WarningsAsErrors.

---

Some default checks appear to be aliases: omit?

Duplicate checks (e.g. cert-con54-cpp vs. cert-con36-c) are specified by the grep pattern cert-*. If we want to deduplicate the set of warnings enabled, we will need to use a different grep pattern than the current default. For example, replacing cert-* with bugprone-*, misc-*, readability-*, etc. Unless we want to reduce the default set of warnings to be more specific, having duplicate checks should be benign.

Opinion: enable WarningsAsErrors for default checks that do not already warn.

Done: WarningsAsErrors is set to '*' (upgrades all warnings listed by Check into errors) with exclusions (-name) for currently-emitted warnings.

The warnings suggested to be enabled are fixed (as described below) and removed from the list of exclusions.

cert-oop54-cpp

Fixed. Deliberately ignored existing inconsistencies between the exception strategy for v_noabi assignment operators given an "invalid" assign-or-destroy-only argument.

cppcoreguidelines-init-variables

Fixed. Used = {} as the universal zero-initializer where auto var = init; is not applicable. Also reduced scope of nearby local variables and applied const-correctness when able. I believe this check will help avoid UB, though perhaps -ftrivial-auto-var-init=pattern might be preferable in the future (in the spirit of P2795R5 "erroneous behavior" for C++26, rather than -ftrivial-auto-var-init=zero as was proposed by P2723R1).

cppcoreguidelines-prefer-member-initializer

Fixed.

cppcoreguidelines-pro-type-static-cast-downcast

This is not a bug due to the cast being guarded by type() == MONGOC_INDEX_STORAGE_OPT_WIREDTIGER. However, replaced the type() check with a dynamic_cast anyways for improved safety (the static cast would be UB for any other class deriving base_storage_options whose type() returns MONGOC_INDEX_STORAGE_OPT_WIREDTIGER). The base_storage_options API is deprecated API anyways; I do not expect this change will make much of a difference, but I agree that this is a useful check to enable nevertheless.

cppcoreguidelines-use-default-member-init

Fixed. Used the same = {} pattern for all trivial data members (as done for cppcoreguidelines-init-variables) and removed superflous member initializers in the ctor.

cppcoreguidelines-non-private-member-variables-in-classes

This is a new clang-tidy warning (already enabled by the cppcoreguidelines-* pattern) emitted by changes merged from #1513 for the mongocxx::v1::pipeline::impl class:

warning: member variable '_count' has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]
warning: member variable '_doc' has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]

This diagnostic seems to warn only when there are both public and private data members in the same class. Because this impl class indeed maintains its own invariants, address this warning by making all the data members private and exposed the scoped_bson _doc; data member via a const-only interface.

misc-throw-by-value-catch-by-reference

This is a new clang-tidy warning (already enabled by the cert-* pattern) emitted by changes merged from #1501 for the mongocxx::v1::throw_exception() helper function:

error: throw expression should throw anonymous temporary values instead [cert-err09-cpp,cert-err61-cpp,-warnings-as-errors]
  336 |     throw ex;
      |           ^

If we want to restrict this warning to identifying non-reference catch statements (which may slice the exception object; this seems to be the main purpose of this diagnostic), we could disable the "throw anonymous temporaries" aspect of this diagnostic by setting the cert-(err09|err61)-cpp.CheckThrowTemporaries option to 'false'. However, I've opted to address the warning by using std::move(ex) instead, as is permitted and described by docs:

Moved named values will not be flagged as not throwing an anonymous temporary. In this case we can be sure that the user knows that the object can’t be accessed outside catch blocks handling the error.

---

Following these changes, the warnings which are currently enabled (by default) but not treated as errors (excluded) are:

• Total warnings emitted: 264 (patch)
• Warnings sorted from top to bottom by highest unique occurance count:
  • cppcoreguidelines-pro-type-union-access: 87 warnings
  • cppcoreguidelines-owning-memory: 39 warnings
  • cppcoreguidelines-pro-bounds-array-to-pointer-decay: 34 warnings
  • cppcoreguidelines-avoid-magic-numbers: 19 warnings
  • cppcoreguidelines-pro-type-reinterpret-cast: 19 warnings
  • cppcoreguidelines-pro-bounds-pointer-arithmetic: 17 warnings
  • cppcoreguidelines-pro-type-const-cast: 13 warnings
  • cppcoreguidelines-pro-type-member-init: 11 warnings
  • cppcoreguidelines-avoid-c-arrays: 9 warnings
  • cppcoreguidelines-use-enum-class: 5 warnings
  • cppcoreguidelines-rvalue-reference-param-not-moved: 3 warnings
  • cppcoreguidelines-avoid-non-const-global-variables: 2 warnings
  • cppcoreguidelines-pro-bounds-constant-array-index: 2 warnings
  • cppcoreguidelines-pro-type-vararg: 2 warnings
  • cert-err58-cpp: 1 warning
  • cppcoreguidelines-avoid-do-while: 1 warning

All other currently-enabled warnings are now treated as errors. Note that with the current arrangement, any new warnings added to the list of diagnostics supported by clang-tidy which match the given patterns will trigger EVG task failure until the new warnings are fixed or excluded.

[connorsmacd]

In my previous role, I took on a similar initiative to gradually introduce more strict clang-tidy checks. From memory, I believed we enforced the majority of the checks from the following groups:

• bugprone
• clang-analyzer
• concurrency
• cppcoreguidelines
• misc
• modernize
• objc
• performance
• portability
• readability

I chose most of these groups as they seemed appropriate for general application rather than being tied to a specific set of company conventions, project conventions, or coding standards. There were two exceptions however:

• cppcoreguidelines: While I do not treat the core guidelines as dogma, they are about the closest thing to a standard set of conventions that have achieved consensus among the C++ community.
• objc: My former codebase had a fair amount of Objective-C code, which I do not believe applies here.

While all these groups were enforced, some checks were selectively disabled from each group due to their enforcement being impractical, overly opinionated, or unreliable (too many false positives). That said, I made a point to document each disabled check to discourage others from disabling checks out of convenience or expediency, e.g.:

Checks:
    - bugprone-*
    - -bugprone-exception-escape # Difficult to enforce since many STL functions lack proper conditional noexcept.
    - readability-*
    - -readability-identifier-length # Too opinionated.

Some of the enforced checks still needed to be suppressed. I similarly tried to encourage devs to document suppressions with some sort of justification, e.g.:

// ABI backward compatibility. Const is restored in `view::_init`.
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
_init(const_cast<void*>(static_cast<void const*>(value)));

While I think this was largely a sound approach, it was also curated for that specific project and domain. Some of these goals may not be as applicable to our use case:

• Encouraging best practices. Though I worked with a team that had good knowledge of C++, I wanted to make sure the codebase was accessible to new hires and/or developers less experienced with C++. As you all know, C++ comes with many "footguns" that may not be obvious to those less familiar with the language. For example, if a someone were to come across a plain const_cast without justification, they may get the impression that a const_cast is a perfectly fine thing to do. Personally, I can remember using a const_cast early in my career just to make a problem go away, which was very much not the right way to solve the problem. One could argue that we should take the same approach for this repo. That said, I would hope that driver maintainers (either us or future maintainers) would have a level of expertise and rigor that would make such misuses highly unlikely.
• Performance considerations. In my previous role, I worked on a high performance computer vision engine, but since the bottlenecks in our system came overwhelmingly from neural network inference and image processing routines, the majority of our code was not performance intensive. Given this, we did not worry about enforcing safety checks, such as cppcoreguidelines-pro-bounds-avoid-unchecked-container-access, that would incur a runtime cost. An open source library that can be used in an unbounded number of applications may not have the same luxury.
• Access to modern standards and third party libraries. A number of checks can be easily dealt with by using modern C++ facilities provided by recent standards or third party libraries. For example, cppcoreguidelines-pro-bounds-pointer-arithmetic can often be resolved using std::span or gsl::span. In our case, we are still stuck on C++11 (with the exception of the std::string_view and std::optional polyfill) and we have a limited ability to introduce third party dependencies. For this warning in particular, we would perhaps have to provide our own implementation of span, which is doable, but requires more work for us up front.

Overall, I am still partial to a somewhat aggressive use of clang-tidy, but I recognize that I am new to this project and have a background in a different domain.

With the amount of checks there are, I cannot practically give my opinion on each one. For now, I will just give my thoughts on some of the warnings with a high occurrence:

• cppcoreguidelines-pro-type-union-access: I imagine a lot of this comes from bson_value_t::value. We cannot simply avoid accessing this union or any other union that comes from the C driver. The best we can do is have single points of access in functions that wrap these unions in something like std::variant, which we don't have.
• cppcoreguidelines-owning-memory: I believe the only way to deal with this warning is to use gsl::owner, which is just a pointer alias. I do not imagine we want to introduce a dependency on GSL. In any case, we are wrapping a C API here, so owning memory is to be expected.
• cppcoreguidelines-pro-bounds-array-to-pointer-decay: This one is complicated. Some cases can be resolved with explicit pointer casts (e.g., char[] -> char const* for C-style strings). Other cases may require something like std::span. It is hard to give a recommendation without digging into these warnings further.
• cppcoreguidelines-avoid-magic-numbers: Generally a good idea, but it can be annoying in tests.
• cppcoreguidelines-pro-type-reinterpret-cast: Recommend enforcing this with suppressions as needed.
• cppcoreguidelines-pro-bounds-pointer-arithmetic: Would be much easier to enforce with std::span.
• cppcoreguidelines-pro-type-const-cast: Recommend enforcing this with suppressions as needed.
• cppcoreguidelines-pro-type-member-init: Recommend enforcing.
• cppcoreguidelines-avoid-c-arrays: Recommend enforcing since we have std::array.
• cppcoreguidelines-use-enum-class: Recommend enforcing unless fixes would cause API breaks.

For the rest, there are so few warnings that it seems practical to enforce then suppress as needed.

I mentioned some other groups of checks that I would support enabling (see the beginning of this comment). To be clear, I think it makes sense to stick with cppcoreguidelines and cert for this PR. In the future, we could consider auditing these additional groups then creating similar discussion PRs like this one.

[eramongodb]

Important

The following checks have been disabled:

• cppcoreguidelines-pro-type-union-access (C++17)
• cppcoreguidelines-pro-bounds-array-to-pointer-decay (noise)
• cppcoreguidelines-pro-bounds-pointer-arithmetic (noise)
• cppcoreguidelines-pro-bounds-constant-array-index (noise)

See below for rationale.

---

Overall, I am still partial to a somewhat aggressive use of clang-tidy [...] I think it makes sense to stick with cppcoreguidelines and cert for this PR. In the future, we could consider auditing these additional groups then creating similar discussion PRs like this one.

Agreed. The focus of this PR is specifically to determine the checks which will trigger EVG clang-tidy task failures (CI), similar in spirit to the set of -Werror flags which are unconditionally set for the C Driver here. To avoid CI noise, I believe it is best to start with a subset of warnings and gradually expand from there as we decide which may be worth treating as unconditional errors. This will almost certainly be a smaller subset of diagnostics than what individual developers can/should enable for local development, which can be controlled via IDE settings and custom ClangTidy configuration files.

---

cppcoreguidelines-pro-type-union-access

These warnings are emitted by both C and C++ Driver code:

• bsoncxx::v1::types::value (members of bson_value_t _value via impl::v())
• bsoncxx::v1::types::view (members of bson_value_t const& v)
• bsoncxx::v_noabi::types::view (union data members)
• mongocxx::v_noabi::model::write (union data members)

In my opinion, and as you've stated, it is not feasible to address these warnings without raising our minimum C++ standard to C++17 or newer to obtain access to std::variant. Therefore, I think it is safe to disable this check for now and revisit re-enabling this check once the minimum C++ standard is updated accordingly.

cppcoreguidelines-owning-memory

I agree we do not want to depend on GSL. The C API is not actually such a concern (since deallocation is managed by bson_free() and similar). These warnings are emitted by:

• explicit delete[] to deallocating owning BSON byte arrays by bsoncxx::(v_noabi|v1)::(array|document)::value.
• explicit delete to deallocate impl objects in v1 API (due to use of void* _impl; data members rather than std::unique_ptr<impl>).

I think this diagnostic is useful to keep enabled as a helpful reminder to use smart pointers when able. Therefore, opted to suppress all current instances of this warning with either "custom deleter" or "owning void* for ABI stability" as rationale. All other cases can, should, and do use std::unique_ptr<T>.

cppcoreguidelines-pro-bounds-array-to-pointer-decay

I find the value of this diagnostic dubious. Even if we go out of our way to explicitly cast C arrays into pointers via static casts... for what benefit? Even if we convert these instances into std::span<T>, we would have to explicitly opt-into bounds-checking by using ::at() rather than ::operator[], which is the same problem that exists for std::vector<T> or std::string subscripting and equally "unsafe" (UB for out-of-range indexes). I think there might be value to this warning if it could be restricted to passing arrays as a pointer to a function without a corresponding size argument (a hypothetical cppcoreguidelines-pro-bounds-array-to-pointer-without-size diagnostic), but this is not the case: this diagnostic warns for any array-to-pointer decay regardless of context (even calls to std::memcpy()!). I believe cppcoreguidelines-avoid-c-arrays (already enabled and mentioned below) provides better value with less noise and uncertainty.

Therefore, opted to disable this check together with related checks cppcoreguidelines-pro-bounds-pointer-arithmetic and cppcoreguidelines-pro-bounds-constant-array-index. We can consider re-enabling these if there are arguments against this decision.

cppcoreguidelines-avoid-magic-numbers

I agree that this warning "can be annoying in tests", but for the moment, these clang-tidy diagnostics only apply to library code. It does not apply to tests, examples, and etc. Therefore, I think this is a useful diagnostic to keep enabled, if only to encourage descriptive names (fix) or documentation (suppression). Therefore, opted to suppress this diagnostic on a case-by-case basis. I think the corresponding changes are indeed a readability improvement. However, I'm not sure if we want to promote this into an error yet, so left it as a warning-only for now.

cppcoreguidelines-pro-type-reinterpret-cast

I agree we should enforce this diagnostic. Fixed and suppressed accordingly.

cppcoreguidelines-pro-type-const-cast

I agree we should enforce this diagnostic. Fix and suppressed accordingly.

The use of Scott Meyers' DRY getters pattern is used by several v_noabi components, but these are not carried over into the v1 API or preserved by v_noabi refactors. Nevertheless, decided to fix these diagnostics for the sake of this PR using a function template instead of const_cast (in the spirit of C++23's "Deducing this" feature).

Fixing these warnings also highlights an inconsistency in the v_noabi::client_session::_get_impl() function which, unlike other _get_impl() functions, does not throw an exception for an assign-or-destroy-only state (there is no v_noabi::error_code::k_invalid_client_session_object) despite 1) clearly matching the pattern for class which do throw, and 2) contrary to this comment suggesting otherwise, a null _impl pointer being possible (move SMFs are defaulted, not deleted). This discrepancy has been present since the commit which first introduced this code. I've opted to copy over the v1 API's postcondition documentation for v_noabi::client_session to highlight this discrepancy in behavior from most other v_noabi classes (treating as a documentation bug). This documentation will carry over into this component's v_noabi refactor (given v1 API consistently uses and documents the "assign-or-destroy-only state" for moved-from objects).

cppcoreguidelines-pro-type-member-init

I agree we should enforce this diagnostic. Fixed and suppressed accordingly.

We could consider setting cppcoreguidelines-pro-type-member-init.IgnoreArrays: 'true' so this diagnostic is not emitted for array members (i.e. _storage, _bytes, etc.), but opted to leave this option as-is for now.

cppcoreguidelines-avoid-c-arrays

I personally think this diagnostic somewhat dubious in value, despite my remarks above concerning cppcoreguidelines-pro-bounds-array-to-pointer-decay. Nevertheless, enforced as suggested. Annoyingly, this diagnostic is also emitted for situations like std::unique_ptr<T[]>. Decided to suppress these cases with a note documenting the use of "fixed-size dynamic array" and the name of the variable by which the size is (already) tracked.

cppcoreguidelines-use-enum-class

I agree we should enforce this diagnostic, although I really wish we could use C++20's using enum E; in some contexts. Fixed and suppressed accordingly.

Also opted to set cppcoreguidelines-use-enum-class.IgnoreUnscopedEnumsInClasses: 'true' to handle cases where an unscoped (and often unnamed) enumeration is used to define compile-time constants within class scope.

cppcoreguidelines-rvalue-reference-param-not-moved

I believe this is worth enforcing, so drive-by fixed these warnings as well.

---

Following these changes, the warnings which are currently emitted but not treated as errors are only six in total:

src/mongocxx/lib/mongocxx/v1/instance.cpp:53:17: warning: variable 'instance_state' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
   53 | std::atomic_int instance_state{0};
      |                 ^
src/mongocxx/lib/mongocxx/v1/pipeline.cpp:50:19: warning: do not call c-style vararg functions [cppcoreguidelines-pro-type-vararg]
   50 |             (void)std::snprintf(_buffer.data(), _buffer.size(), "%" PRId32, _count);
      |                   ^
src/mongocxx/lib/mongocxx/v_noabi/mongocxx/gridfs/bucket.cpp:270:5: warning: avoid do-while loops [cppcoreguidelines-avoid-do-while]
  270 |     do {
      |     ^
src/mongocxx/lib/mongocxx/v_noabi/mongocxx/instance.cpp:42:24: warning: variable 'current_instance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
   42 | std::atomic<instance*> current_instance{nullptr};
      |                        ^
src/mongocxx/lib/mongocxx/v_noabi/mongocxx/instance.cpp:63:13: warning: do not call c-style vararg functions [cppcoreguidelines-pro-type-vararg]
   63 |             mongoc_log(MONGOC_LOG_LEVEL_INFO, "mongocxx", "libmongoc logging callback enabled");
      |             ^
src/mongocxx/lib/mongocxx/v_noabi/mongocxx/uri.cpp:50:24: warning: initialization of 'k_default_uri' with static storage duration may throw an exception that cannot be caught [cert-err58-cpp]
   50 | std::string const uri::k_default_uri = "mongodb://localhost:27017";
      |                        ^

I am in favor of enforcing cppcoreguidelines-avoid-non-const-global-variables and cert-err58-cpp. I don't think we can avoid cppcoreguidelines-pro-type-vararg (disable? suppress?). I don't know how I feel about cppcoreguidelines-avoid-do-while.