Reduce Warnings: miscellaneous MSVC warnings (CDRIVER-5981)

[eramongodb]

This PR in total addresses 3573 (1192 unique) warnings with VS 2022 (version 17.13.6) and MSVC 19.43.34810.

Following the changes in this PR, the only remaining Level 4 warnings observed are related to signedness inconsistency (C4245, C4267, C4389), implicit conversions (C4244), and type casts (C4057). This brings the quality of emitted MSVC warnings to a state that is comparable to that of GCC and Clang, whose currently-emitted warnings are also largely related to signedness consistency and implicit conversion warnings.

Note

Several attempts were made to address the GCC -Wtype-limits warnings emitted by the (unsigned) < 0 comparison by mlib_upsize_integer in intutil.h, but was unable to suppress the warning due to unexpected parser errors.

---

Addresses 801 (100 unique) C4146 "unary minus operator applied to unsigned type, result still unsigned" warnings.

Nearly all of these warnings are due to the deliberate use of the unsigned unary minus operator in mlib_minof macro (typically via the mlib_in_range macro). Because this is deliberate, this PR suppresses this warning.

Important

Because the mlib_minof and mlib_in_range macros are used as expressions rather than statements, the usual mlib_* diagnostic macros are not usable due to forced trailing semicolons. For MSVC, __pragma is used explicitly and guarded by MLIB_IF_MSVC (_Pragma requires /std:c11 or newer). This is also the case for other instances of explicit __pragma instead of mlib_* diagnostic macros for warnings described below.

The only other instance of this warning is in bson-vector.c in a deliberate bitwise computation of padding.

---

Addresses 746 (633 unique) C4189 "local variable is initialized but not referenced" warnings.

These warnings have two categories.

The first category are warnings emitted in the following confusing form:

'$S1234': local variable is initialized but not referenced

These warnings appear to be referencing the initialization of data members of temporary structs (denoted as $S1234 for some number 1234). Most of these warnings are due to the mlib_upsize_integer macro. Inline suppressions are added accordingly.

The second category are due to the BSON_MAYBE_UNUSED macro, used by the BSON DSL macros, having no effect on MSVC due to lack of a feature equivalent to __attribute__((unused)). MSVC only supports [[maybe_unused]] for VS 2017 15.3 and newer with the /std:c++17 flag, which is irrelevant to our codebase. Therefore, the BSON_MAYBE_UNUSED macro is replaced with the familiar (void) var; workaround to silence unused warnings.

---

Addresses 126 C4210 "nonstandard extension used: function given file scope" warnings (124 unique).

These warnings are related to an old MSVC language extension where functions declarations within function scope have file scope instead. Most of these warnings are due to TEST_INSTALL, with the remainder pertaining to the mongoc_get_init_called function. Due to the limited scope and impact of these warnings, they are either directly silenced using mlib_msvc_warning or the declaration simply moved out of function scope into file scope.

---

Addresses 6 (2 unique) C4200 "nonstandard extension used: zero-sized array in struct/union" warnings.

These warnings appear to be false-positives caused by use of a flexible array member in jsonsl_st and pool_node. Both are using an incomplete array type T member[]; to declare a flexible array member per the C99 standard; they are not declaring a zero-size array T member[0]; as C4200 implies. These warnings are therefore suppressed.

---

Addresses 8 (4 unique) C4232 "nonstandard extension used: 'fn': address of dllimport 'fn' is not static, identity not guaranteed" warnings.

These warnings are related to an old MSVC language extension permitting one to evaluate the address of a __declspec(dllimport)-ed function (address is computed at runtime) at compile-time via static initialization (address is computed at compile-time). In our case, these warnings are due to initializing gMemVtable's function pointers to their default value: malloc, calloc, realloc, and free. Because we do not expect users or ourselves to depend on the stability of the address of these stdlib allocation functions, this PR opts to simply silence this warning without making significant changes.

---

Addresses 1350 (91 unique) C4100 "unreferenced formal parameter" warnings.

These warnings have three categories.

The first category are simply parameters which are not referenced either due to obsoletion or conditional compilation. These warnings were addressed by either adding BSON_UNUSED(param); or by removing the parameter altogether.

The second category are due to "call once" functions containing unused parameters when built with MSVC to satisfy InitOnceExecuteOnce API requirements. These warnings are suppressed accordingly using MLIB_IF_MSVC (__pragma (...)) to avoid interfering with the function declaration syntax.

The third category are (pointer) parameters which are referenced, but only by the BSON_OPTIONAL_PARAM macro. These were addressed by changing the definition of BSON_OPTIONAL_PARAM from a no-op to ((void) param).

---

Addresses 207 (161 unique) C4456 "declaration of 'var' hides previous local declaration" warnings.

Nearly all of these warnings were due to BSON DSL macros, which reuse the same local variable names within nested scopes according to how the DSL macros are used and expanding. The existing suppression of -Wshadow warnings for GCC and Clang were extended to also suppress C4456.

The only other change required to address this warning was in test-bson.c, which only required a trivial rename.

---

Addresses 1 C4308 "negative integral constant converted to unsigned type" warning.

Although this is normally something to be avoided, this case was a deliberate sanity check ensuring signed op unsigned is implicitly converted to unsigned op unsigned prior to further evaluation as expected, rather than implicitly converting to signed op signed.

---

Addresses 4 (2 unique) C4459 "declaration of 'var' hides global declaration" warnings.

These were all due to the redundant redefinition of the message_header_length constant within function scope in mongoc-cluster.c. The redundant definitions are removed accordingly.

---

Addresses 2 (1 unique) C4132 "const object should be initialized" warnings.

These warnings are due to the gZero static const variable lacking an explicit initializer in its definition. Even though it is well-defined to perform zero-initialization, there is no harm in being explicit about it, so a = 0 initializer was added to the definition to address this warning.

---

Addresses 3 unique C4996 "Hedged Reads are deprecated in 8.0" warnings.

These warnings appear to have been overlooked by #1889. These are addressed with straightforward use of the mlib_disable_deprecation_warnings() diagnostic macro.

---

Address C4702 "unreachable code" warnings.

These warnings have two categories.

The first category are "expanded" or "explanatory" code which were not commented out in mlib/ckdint.h. These code blocks were converted into comments accordingly.

The second category are macro expansions which happen to containing unreachable code paths due to build configuration (HASH_ADD_KEYPTR) or expanded code branches (BSON DSL macros). In both cases, there is an expanded call to abort() which makes subsequent evaluation of internal code unreachable (e.g. __bson_dsl_indent;). Attempting to address these warnings were frustrated by the following quirk:

Warning numbers in the range 4700-4999 are associated with code generation. For these warnings, the state of the warning in effect when the compiler reaches the function definition remains in effect for the rest of the function. Use of the warning pragma in the function to change the state of a warning number larger than 4699 only takes effect after the end of the function.

Accordingly, attempts to insert suppressions within the relevant BSON DSL macros failed. There was no choice but to silence the warning at the usage sites instead (at file scope to completely surround relevant function definitions).

---

Addresses 4 unique C4703 "potentially uninitialized local variable" warnings.

Although most of these warnings simply required an explicit initializer to address, a couple cases involved some non-straightforward control flow and branch guards. Comments were added in addition to the initializer to hopefully provide clearer context.

---

Addresses 1 unique C2099 "initializer is not a constant" warning.

This one is actually not our fault. There is a regression in the Windows 11 SDK where the NAN macro constant is no longer a compile-time constant. This regression is yet to be fixed (but planned for a late April release). Declaring the array as a normal function-local variable is sufficient to workaround this issue.

[eramongodb]

Investigating syntax errors with VS 2015 due to __pragma expansion for call once functions.

[eramongodb]

Investigating syntax errors with VS 2015

Fixed. Appears to have been due to a poor interaction between the placement of the __pragma relative to the CALLBACK specifier.