CXX-3072 Reduce Warnings - Code Consistency and Quality Improvements

[eramongodb]

Summary

Resolves CXX-3072. Verified by this patch.

Similar to mongodb/mongo-c-driver#1543 for the C Driver. Addresses 615 Clang warnings (83 unique warnings) with Clang 19.

scoped_bson_value

A long overdue followup to #955:

It may be worth eventually extracting this helper into a proper a proper component (probably under src/mongocxx/private?), but that work has been deferred for now.

This addresses -Wunused-member-function warnings in options.cpp due to unused functions with internal linkage (default ctor + value_for_init).

C-Style Casts

A large part of this PR is devoted to addressing -Wold-style-cast warnings.

Most cases only needed a trivial change from (T)expr to static_cast<T>(expr). Regarding numeric casts, range checks have not been added: those may be audited in the future when an assertion/contract methodology for the C++ Driver is implemented (directly reusing C Driver's BSON_ASSERT() does not seem ideal).

A notable change is the use of const_cast to workaround the void* parameter (lack of const) in bsoncxx::v_noabi::types::bson_value::view's private member functions. To avoid unnecessarily breaking ABI compatibility, a const_cast is used instead of updating the parameter type, accompanied by a comment documenting the context and its validity (the const is recovered in the _init function + corresponding convert_from_libbson() overloads updated with the missing const for consistency).

Miscellaneous

Other warnings addressed by this PR include:

• handling of semicolons (both removing redundant ones and adding some following macro use for consistency)
• missing case statements in switches on enumerations (in error_code.cpp in particular).
  • is the write_concern::level::k_acknowledged do-nothing fallthrough in write_concer.cpp correct?
• #if defined(X) instead of #if X when X is not guaranteed to be defined.
• static -> inline for function definitions in headers used by multiple components (where internal linkage does not seem sufficiently motivated).
• NULL -> nullptr even with C APIs (C idioms shouldn't leak into C++ code).
• 0 -> nullptr in pointer comparisons and initialization.

Ignored Warnings

The following warnings were observed but deliberately unaddressed/ignored/silenced:

• -Wdocumentation: addressed by CXX-3070 Address -Wdocumentation Clang warnings #1175.
• -Walign-mismatch in bsoncxx/v_noabi/bsoncxx/private/stack.hh: unclear how to resolve this warning.
• -Wexit-time-destructors for exception types: deferred to a future API major release.
• -Wglobal-constructors + -Wexit-time-destructors for uri::k_default_uri: deferred to a future API major release.
• -Wundefined-func-template for soft_declval: deferred to v1 migration PRs (CXX-2745).
• -Wweak-vtables for exception types: deferred to a future API major release.

[Julia-Garland]

Besides not being sure about your question on write_concern::level::k_acknowledged LGTM!

[Julia-Garland]

is the write_concern::level::k_acknowledged do-nothing fallthrough in write_concern.cpp correct?

I checked out the API reference and the MongoDB docs, not sure myself.

[kevinAlbs]

I expect this case is unreachable. If this else block is entered, then nodes() did not return a value. That implies acknowledge_level() cannot be k_acknowledged.

I expect this could assert:

            case write_concern::level::k_acknowledged:
                // Acknowledged write concern is expected to return `nodes()` above.
                MONGOCXX_UNREACHABLE;

[eramongodb]

Given prior default: behavior, opting to preserve break; behavior rather than introducing an assertion out of caution given the somewhat indirect relationship between is_acknowledged() and get_w(). However, will include a comment documenting the relationship with the returned optional.

[kevinAlbs]

The warning reductions are appreciated. LGTM with a possible use of BSONCXX_UNREACHABLE.

[kevinAlbs]

I expect this case is unreachable. If this else block is entered, then nodes() did not return a value. That implies acknowledge_level() cannot be k_acknowledged.

I expect this could assert:

            case write_concern::level::k_acknowledged:
                // Acknowledged write concern is expected to return `nodes()` above.
                MONGOCXX_UNREACHABLE;