This extends the check for default initialization in arrays added in
547f89d6070 to include scalar types and exclude them from the suggested fix for
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.
Drop the const, we don't use const on local variables in the codebase.
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.
Understood, but replacing std::unique_ptr<int>(new int) with std::make_unique<int>() does change program behavior. Once the code migrates to value initialization, it can be hard to determine later whether that initialization ends up being unnecessary when C++20 or an analogue for std::make_unique_for_overwrite is available.
I'm hesitant to nudge users towards the behavior change when alternatives would be available (maintain the status quo, have C++20, preadopt a make_unique_for_overwrite helper like we do with this clang-tidy for pre-C++14 builds).
I think for starters this patch needs a better description, explaining not *what* it does, but *why* it does what it does.
Also, it is probably not good to change (break) existing test coverage.
Instead, it is best to add new tests, and adjust existing check lines.
std::make_shared<int>() is equivalent to std::shared_ptr<int>(new int()). This value initializes (read: zeroes out the value).
The statement here is only default initializing (read: leaves the value uninitialized). For this use case, we should use std::make_shared_for_overwrite when it is available.
I see. Did you consider an option to transform these anyway? It seems at least as likely that for real-world code bases they are not initialized accidentally (as opposed to being a deliberate choice as assumed here).
The default initialization case for arrays was added because it caused several performance regressions. I want to get the complete initialization check implemented, which will then make the make_unique_for_overwrite rewrite straight forward (just !initializating).
An option could then be added in a subsequent commit.