[PATCH] D78546: Enable bugprone-argument-comments check in llvm.

[Sam McCall via Phabricator via llvm-commits]

sammccall added a comment.

> LLVM code style encourages this

Not very strongly, it just says it can be helpful (and tells you how to do it if you do it). So I'm wary of making it mandatory in cases where it isn't a clear readability win.

Had a look at this on clangd code:

- the basic no-options functionality (wrong name for comment) looks good - some of them don't matter much, but didn't find any where the fix made it worse
- the single-argument cases were almost all false positives, that flag should be flipped
  - but it seems to count parameters rather than arguments so `foo.substr(1)` still fires
- this is no good for standard library functions, because (at least with libstdc++):
  - there are leading underscores in the names, these are ignored for matching but included in suggestions
  - the standard doesn't really define the names (at least implementations don't match cppreference) so the check results aren't portable
  - e.g. it fired on basic_string(/*Repeat=*/..., 'x') and wanted Repeat replaced with `__n`.
- things of the form `range(0, foo.size())` => `range(/*Start=*/0, foo.size())` seem unhelpful

I think this would be a better check if we **increased the arg threshold to 3** and **excluded functions in namespace std**.
As it is I'm not sure whether it does more good than harm. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78546