[PATCH] D121214: [clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

I have some concerns about this. While it is now clear to me that the //partial//ness refers to partial coverage of the guideline rule, it is indeed very, very partial. **MEM51-CPP** as of now lists **9** cases of non-compliant examples, from which `std::shared_ptr<T> = new S[8];` is just //one//. `bugprone-shared-ptr-array-mismatch` (D117306 <https://reviews.llvm.org/D117306>) in itself is a check that inherits from a "more generic" smart pointer check.

Right now, there is no check for the exact same `unique_ptr` case, which the linked CERT site explicitly phrases:

> As with std::unique_ptr, when the std::shared_ptr is destroyed [...]

Let's suppose a `bugprone-unique-ptr-array-mismatch` check is also created (even though that would mean immediately that the naming of the `shared_ptr` one is kind of bad and that we should rename or deprecate the check... which is its own can of worms with regards to support of clients...). What will this alias become, then?

At first re-read my recent comment of

In D121214#3367609 <https://reviews.llvm.org/D121214#3367609>, @whisperity wrote:

> Is "partial aliasing" a new notion?

sounded weird to me too, but thinking this through again, I think I know what I was thinking yesterday.

Please correct me if I'm wrong, but the "name" of a check (alias or not) is a **unique** property. We lack the infrastructure support for `1:N` aliasing. While there are certainly cases where we only managed to partially cover a rule, and made an alias for it, missing a small chunk of the guideline (e.g. because static analyses can't catch such) looks less intrusive than -- essentially -- //"trying to sell 1/9 coverage as **the** alias"//.



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-mem51-cpp.rst:12
+
+Please note that the check only partially covers the corresponding CERT rule.
----------------
//Maybe// this warning could be emphasised with the appropriate RST entity, but a quick grep of Tidy docs turned up no such similar constructs so maybe it is better if we do not break form here...




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121214/new/

https://reviews.llvm.org/D121214