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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to