-
Notifications
You must be signed in to change notification settings - Fork 543
[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
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_X
macros 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 |
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_unique
with our own implementation. This also includes some aside changes related to CMake target scripting.