For a c++11 code, the clang-tidy rule "modernize-make-unique" should return immediately, as std::make_unique is not supported.
hokein aaron.ballman ilya-biryukov alexfh
- rG670c6315acac: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std…
rCTE328101: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std…
rL328101: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std…
Thanks for the patch!
The check provides MakeSmartPtrFunction option. Users can use it to customize their self-implemented make_unique function (instead of using the c++14 available std::make_unique) even in their C++11 code. I think we need to keep the backward compatibility.
I'd suggest only checking the C++14 flag when the suggested fix is "std::make_unique".
|38 ↗||(On Diff #135912)|
I'd better use positive assertions with the original text. Otherwise the test can pass if the check does any replacements except for the ones you list in CHECK-FIXES-NOT.
|8 ↗||(On Diff #136245)|
As Haojian mentioned, this test still has to work in c++11.
Why is this is a user-facing option?
If it needs to be a user-facing option, you also need to implement an override for storeOptions() as well.
What is this option? Why is this returning a string literal rather than something less error-prone like an enumeration?
As the rule was very customizable, I also made the c++ version customizable. But the option is only useful if a user has a custom make_unique that needs c++14 (uses std::make_unique internally) and multiple versions of C++ in their codeline. I agree this is a rather specific usecase. I can remove it and make the code simpler if it is your recommendation.
I made the MinimumLanguageVersion a string literal to make it customizable by the user. If we remove the customization, we can switch to an enum or a boolean.
The only reason to have a custom make_unique that I know is to workaround the absence of std::make_unique in c++11. So this looks like a not very useful case to support.
From the peanut gallery: IIUC, yes, this is not a useful case to support.
IIUC above, you allow the user to pass in the name of a custom my::make_unique function. Could you add a test case for that?
A couple of nits.
nit: Use ternary operator?
No need to define int main() in the test. It can also become confusing for the readers of the code. Please change this to void f() or something like that and remove the return statement.