-
Notifications
You must be signed in to change notification settings - Fork 548
[CXX-2625] Bring our own make_unique #1028
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
[CXX-2625] Bring our own make_unique #1028
Conversation
278ab88 to
38b4684
Compare
38b4684 to
aa8b45a
Compare
kevinAlbs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Impressive. LGTM.
|
|
||
| TEST_CASE("Create a unique_ptr") { | ||
| auto ptr = bsoncxx::stdx::make_unique<int>(12); | ||
| CHECKED_IF(ptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| CHECKED_IF(ptr) { | |
| CHECK(ptr); | |
| CHECKED_IF(ptr) { |
Suggest checking ptr since CHECKED_IF only enters block if the expression evaluates to true.
Suggestion applies to other CHECKED_IF statements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to say that this is desired behavior, but I double-checked the docs and
CHECKED_Xmacros were changed to not count as failure in Catch2 3.0.1.
😐
As someone that has used these macros heavily across dozens projects for several years and seems to have missed this release note, this is an extremely breaking change from Catch2, and there doesn't appear to be a viable alternative?? It seems that I have a lot of tests to go and fix now. I need to go lay down.
| * the length of the array to allocate. | ||
| * | ||
| * Requires: | ||
| * - T must be value-initializable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * - T must be value-initializable | |
| * - T must be default-initializable |
kkloberdanz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
Refer: CXX-2625. This changeset replaces our use of boost's
make_uniquewith our own implementation. This also includes some aside changes related to CMake target scripting.