In D102727#2880396, @ldionne wrote:

@ckennelly I know you did a lot of work on std::string, can you take a look at this change?

Any updates on the shared_ptr work?

Ping

@njames93: Any additional comments?

In D90392#2436639, @hokein wrote:

thanks, I think this patch is good, feel free to land it (and sorry for the delay).

As @lebedev.ri commented, would be nice to mention the motivation of the change (make_shared_for_overwrite?) in the description.

updating commit message

rebase

Ping

Ping

Ping

Ping

Applied review feedback

Added option to allow rewrites for default to value initialization

Ping

Applied review comments

Applied review feedback

Ping

Applied feedback from review

• reduced test cases to verify functionality
• updated release notes
• updated docs

In D91009#2381590, @njames93 wrote:

You don't really need to duplicate every single std::string test. Just enough so that it detects it, kind of like ::out::TestString. Also can you update the docs and release notes to explain this behaviour.

njames93: Ping

Expand tests to cover value and default initialization

Ping

Ping

In D90392#2362327, @lebedev.ri wrote:

In D90392#2362308, @ckennelly wrote:

In D90392#2362118, @njames93 wrote:

IIUC, this is handling the case where Ptr.reset(new int) which is different to Ptr.reset(new int()) because the former doesn't initialise the int while the latter default(zero) initialises it.
If that's correct I still think we should still warn about this case, but don't suggest an auto-fix as that will change behaviour.
Maybe put a note explaining why it can't be auto-fixed.

I disagree with printing a warning but not a fix.

These uses should migrate to std::make_unique_for_overwrite/std::make_shared_for_overwrite. I am planning on sending a follow-up patch for that but want to avoid the existing make-unique/make-shared checks try to migrate default initialization use cases.

Reminder that there is more than one version of C++ standard, and users are not obligated to be using some particular version, and the checks should not be making that decision for user.

In D90392#2362118, @njames93 wrote:

IIUC, this is handling the case where Ptr.reset(new int) which is different to Ptr.reset(new int()) because the former doesn't initialise the int while the latter default(zero) initialises it.
If that's correct I still think we should still warn about this case, but don't suggest an auto-fix as that will change behaviour.
Maybe put a note explaining why it can't be auto-fixed.

Apply review feedback from njames93

Apply feedback from ymandel

The test failures look like unrelated tests with flakes.

Restrict verify tests to C++20

Apply feedback on review

In D90111#2353912, @ldionne wrote:

Thanks a lot for the patch! Do you think you could update the patch with the rest of the paper too? I'm fine reviewing it all in one patch (actually I find it easier to validate that the whole paper is implemented that way).

• Fix synposis
• Restrict tests to C++20

Fixing lint errors

Fix in https://reviews.llvm.org/D85944

Applied edits, and lint errors

We could scan /proc/self/smaps for regions huge page eligibility, but that's probably brittle/annoying.

I can go through the char_traits, but char_traits::compare() has an extra branch that is unnecessary here. compare checks that the size is not 0, which is useful to avoid calling memcmp with null pointers. But there is no possiblity of null pointers here. The .data() will never return null, and the char* can't be null either (there is an assertion for that, and we passed it to strlen).

In D55741#1361040, @jwakely wrote:

I'd be happy to restrict this to > C++17 only (which is automatically the case when using G++ because there's no -fdestroying-delete to enable it, you only get it with -std=c++2a, -std=gnu++2a etc.)

Conditioning off of C++17 (or part C++11 and part C++17) seems reasonable, although we may want consistency with libc++ (discussion on https://reviews.llvm.org/D55840).

In D55840#1348416, @ldionne wrote:

In D55840#1348304, @mclow.lists wrote:

Another question - why do we need to check if clang supports destroying delete to define the type destroying_delete_t? Why not define it always, and just define the feature test macro when we have compiler support?

I'm not a big fan of this approach. It seems better to define __cpp_lib_destroying_delete if and only if we provide destroying_delete_t, which makes sense if and only if the compiler has support for it (i.e. __cpp_impl_destroying_delete is defined).

Yes, I need someone to commit for me.

Added update to cxx_status.html for partial implementation of feature test macros adopted in San Diego

I don't have commit access, so can this be committed to trunk?

In D41746#973941, @EricWF wrote:

This LGTM minus nits.

Is there a LWG issue or paper that specifies this change? or is it just a general bug fix?