Cxx 2496 #907
Merged
Cxx 2496 #907
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #907 +/- ##
===========================================
- Coverage 91.28% 74.22% -17.06%
===========================================
Files 384 384
Lines 23275 23310 +35
===========================================
- Hits 21246 17302 -3944
- Misses 2029 6008 +3979
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
…s for the index as specified in the arguments
|
Not sure what could be causing this warning from the POWER build: |
kkloberdanz
requested review from
kevinAlbs,
eramongodb,
vector-of-bool and
rcsanchez97
rcsanchez97
approved these changes
Dec 15, 2022
I poked around at this for a bit and it isn't clear to me why this is an error on just this particular platform. If you'd like it, I can try to help dig deeper. Let me know. |
Thanks! I appreciate it! I'm not quite sure what's causing it. |
This reverts commit 44b2a5a.
kkloberdanz
commented
Dec 16, 2022
CMakeLists.txt
Outdated
Comment on lines
43
to
45
| if(CMAKE_SYSTEM_PROCESSOR STREQUAL "ppc64le") | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-maybe-uninitialized") | ||
| endif() |
There was a problem hiding this comment.
I'm open to suggestions on where is the best place to put this check.
|
Doesn't `-Wno-maybe-uninitialized` disable the warning entirely? I
would think that `-Wno-error=maybe-uninitialized` is more useful as it
still allows the warnings to be produced but without failing the build.
As far as location, where you have it works perfectly well, or you could
move it down a bit to just before the line with
`set (CMAKE_SKIP_BUILD_RPATH false)`. That would be my preferred
location as this is new check you are adding is definitely the most
obscure of the checks.
|
kevinAlbs
reviewed
Dec 16, 2022
CMakeLists.txt
Outdated
| @@ -40,6 +40,10 @@ else() | |||
| message(WARNING "Unknown compiler... recklessly proceeding without a version check") | |||
| endif() | |||
|
|
|||
| if(CMAKE_SYSTEM_PROCESSOR STREQUAL "ppc64le") | |||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-maybe-uninitialized") | |||
| endif() | |||
There was a problem hiding this comment.
This warning seems more dependent on our test environment, rather than something intrinsic to all builds.
Suggest adding to the cmake_flags of .mci.yml. .mci.yml is the configuration for Evergreen.
# power8_cmake_flags includes -Wno-maybe-uninitialized to ignore a warning from the std::optional implementation of MNMLSTC.
power8_cmake_flags: &power8_cmake_flags -DCMAKE_C_FLAGS="-Wall -Wextra -Wno-attributes -Werror -Wno-missing-field-initializers $ignore_deprecated" -DCMAKE_CXX_FLAGS="-Wall -Wextra -Wconversion -Wnarrowing -Wno-expansion-to-defined -pedantic -Werror -Wno-missing-field-initializers -Wno-aligned-new $ignore_deprecated -Wno-maybe-uninitialized"And use power8_cmake_flags on the Power8 variants.
src/mongocxx/collection.hpp
Outdated
| /// @note | ||
| /// Needed for FLE 2 Support. | ||
| /// | ||
| void drop(bsoncxx::document::view_or_value collection_options); |
There was a problem hiding this comment.
I suggest removing this overload, and adding collection_options as optional argument to the two existing drop overloads as follows:
void drop(const bsoncxx::stdx::optional<mongocxx::write_concern>& write_concern = {},
bsoncxx::document::view_or_value collection_options = {});
void drop(const client_session& session,
const bsoncxx::stdx::optional<mongocxx::write_concern>& write_concern = {},
bsoncxx::document::view_or_value collection_options = {});That matches the pattern of overloads for database::create_collection
Co-authored-by: Kevin Albertson <[email protected]>
Co-authored-by: Kevin Albertson <[email protected]>
Co-authored-by: Kevin Albertson <[email protected]>
…stead of an overloaded function
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CXX-2496
Add FLE 2 behavior for CreateCollection() and Collection.Drop()