Merge rules R.12 and R.13; explain about RAII factories. #1610
Conversation
| @@ -9365,73 +9364,75 @@ If you have a naked `new`, you probably need a naked `delete` somewhere, so you | |||
|
|
|||
| (Simple) Warn on any explicit use of `new` and `delete`. Suggest using `make_unique` instead. | |||
|
|
|||
| ### <a name="Rr-immediate-alloc"></a>R.12: Immediately give the result of an explicit resource allocation to a manager object | |||
| ### <a name="Rr-immediate-alloc"></a>R.12: Instead of manual resource allocation, use factory functions that return RAII objects | |||
There was a problem hiding this comment.
This appears to be assuming that you're the author of the factory function in question? What if it's an old third party library? or it's a c library? Or the person next to you wrote it and you don't want to have to go and change all the places it's already called?
There was a problem hiding this comment.
| @@ -9365,73 +9364,75 @@ If you have a naked `new`, you probably need a naked `delete` somewhere, so you | |||
|
|
|||
| (Simple) Warn on any explicit use of `new` and `delete`. Suggest using `make_unique` instead. | |||
|
|
|||
| ### <a name="Rr-immediate-alloc"></a>R.12: Immediately give the result of an explicit resource allocation to a manager object | |||
| ### <a name="Rr-immediate-alloc"></a>R.12: Instead of manual resource allocation, use factory functions that return RAII objects | |||
There was a problem hiding this comment.
The point of R.12 is exactly about calling C/POSIX/WinAPI/etc "explicit resource allocation" interfaces.
There was a problem hiding this comment.
There was a problem hiding this comment.
That line is a footnote on what is shaping up to be a different rule.
There was a problem hiding this comment.
...that is not to say it's wrong, but perhaps it needs to be more obvious that it covers the old cases of making calls to explicit APIs as the old example showed.
There was a problem hiding this comment.
"That line is a footnote on what is shaping up to be a different rule." — Eh? The text I was trying to point to is new in this pull request, and belongs to rule R.12: Instead of manual resource allocation, use factory functions that return RAII objects. Here it is:
If there is no factory wrapper for your RAII type yet, write one!
void bad(const char *name)
{
FILE *f = fopen(name, "r"); // bad
AutoFclosingFile myfile(f);
// ...
}
void good(const char *name)
{
AutoFclosingFile myfile = my::open_file(name, "r");
// ...
}
Nobody should ever be calling "explicit resource allocation interfaces" directly in user code (conflating their actual business logic with manual resource management); that would violate rules F.2 and F.3 and perhaps F.1. Resources should be managed by resource-management classes.
There was a problem hiding this comment.
I'm a fan of collapsing of the rules, the "write a wrapper" is what I always call for.
R.12 and R.13 were both written for a C-style world where the idea was to get raw resource pointers and then quickly "give" them to RAII types. The modern C++ approach is actually to use RAII types exclusively, and never handle raw resource pointers even for an instant. R.13 was kind of getting there, in that it knew `make_shared` existed; but we can strengthen the rule by just saying "Please, use `make_shared`." "Raw pointers are like raw meat: don't touch them with your hands." Also update for C++17: it's no longer true that `shared_ptr<T>(new T())` can ever cause a leak due to interleaved evaluations. But it's still a nasty habit. Use `make_shared`.
R.12 and R.13 were both written for a C-style world where the idea was
to get raw resource pointers and then quickly "give" them to RAII types.
The modern C++ approach is actually to use RAII types exclusively,
and never handle raw resource pointers even for an instant. R.13 was
kind of getting there, in that it knew
make_sharedexisted; but wecan strengthen the rule by just saying "Please, use
make_shared.""Raw pointers are like raw meat: don't touch them with your hands."
Also update for C++17: it's no longer true that
shared_ptr<T>(new T())can ever cause a leak due to interleaved evaluations.
But it's still a nasty habit. Use
make_shared.