Skip to content

CXX-3253 reintroduce VS 2015 task coverage #1381

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

Merged
merged 13 commits into from
Apr 21, 2025

Conversation

eramongodb
Copy link
Contributor

Resolves CXX-3253. Followup to mongodb/mongo-c-driver#1991.

@eramongodb eramongodb requested a review from kevinAlbs April 11, 2025 21:36
@eramongodb eramongodb self-assigned this Apr 11, 2025
@eramongodb eramongodb removed the request for review from kevinAlbs April 11, 2025 21:47
@eramongodb
Copy link
Contributor Author

Converting to draft due to discovery of overlooked EVG configuration errors. Will fix and re-request reviews when ready.

@eramongodb eramongodb marked this pull request as draft April 11, 2025 21:48
@eramongodb
Copy link
Contributor Author

eramongodb commented Apr 15, 2025

Identified compile_vars was not being passed to Compile.call() as it should be. This means the REQUIRED_CXX_STANDARD and USE_STATIC_LIBS environment variables were not being applied as they should.

While addressing this bug, took the opportunity to extend compiler coverage to better match that of the C Driver's std-matrix tasks, including coverage with /std:c++latest (which is supported on EVG distros unlike /std:clatest). Rather than separate tasks for shared and static libraries, the BUILD_SHARED_AND_STATIC_LIBS option is set to ON instead.

Both of these updates revealed several issues which are addressed by this PR, as described below.


Identified a few more references to C Driver hedged reads API in test code that were missed in #1351. Applied suppressions accordingly.


The false-positive -Wmaybe-uninitialized warning for optional<T> made a return with GCC 8 and newer. Although it was meant to be suppressed by #968, the same false-positive warnings were (inconsistently) observed in other functions returning an optional<T> as well, e.g.:

uri.cpp: In member function 'std::optional<int> mongocxx::v_noabi::uri::wait_queue_timeout_ms() const':
error: '<anonymous>' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         return {};
                 ^

To avoid this issue for good, changed all instances of the problematic return {}; pattern to return bsoncxx::v_noabi::stdx::nullopt; instead.


Extending compiler coverage to GCC 4.8 (on rhel76) revealed the following compilation error:

error: unknown option after '#pragma GCC diagnostic' kind [-Werror=pragmas]
  BSONCXX_PRIVATE_WARNINGS_DISABLE(GNU("-Wignored-attributes"));

This required adding the defined(__GNUC__) && (__GNUC__ >= 6) && !defined(__clang__) condition to avoid suppressing an unsupported warning flag for GCC versions older than GCC 6.


Following #1329, there should have been no more alignment-related issues in the C++ Driver. However, when removing an overlooked -Wno-aligned-new flag in the compile script, more instances of this warning were identified with Clang 13 (on rhel90), e.g.:

error: passing 4-byte aligned argument to 8-byte aligned parameter 1 of 'bson_get_data' may result in an unaligned pointer access [-Werror,-Walign-mismatch]
    return {bson_get_data(failure), failure->len};
                          ^

Further investigation revealed a common pattern for all instances of this warning: the use of auto to deduce a bson_t const*. This suggests auto is also susceptible to the "attributes are ignored" issue which affects template parameters as described in #1329. Changing all instances of such auto variables to explicit bson_t const* addressed these warnings.

@eramongodb eramongodb marked this pull request as ready for review April 15, 2025 16:09
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

The compile matrix improvements (easier visibility to which compiler version is tested) are very much appreciated. LGTM

@eramongodb
Copy link
Contributor Author

Deferred attempts to resolve rhel76 task failure to #1384.

Copy link
Contributor

@vector-of-bool vector-of-bool left a comment

Choose a reason for hiding this comment

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

LGTM

@eramongodb eramongodb merged commit f6fef68 into mongodb:master Apr 21, 2025
18 of 21 checks passed
@eramongodb eramongodb deleted the cxx-3253 branch April 21, 2025 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants