[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added inline comments.


================
Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:1
-//===--- ArgumentCommentCheck.cpp - clang-tidy ----------------------------===//
 //
----------------
aaron.ballman wrote:
> Why did this get removed?
Sorry I suspect poor vim skills


================
Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:231
+bool ArgumentCommentCheck::shouldAddComment(const Expr *Arg) const {
+  return (CommentBoolLiterals && isa<CXXBoolLiteralExpr>(Arg)) ||
+         (CommentIntegerLiterals && isa<IntegerLiteral>(Arg)) ||
----------------
aaron.ballman wrote:
> I think we may want to do some additional work here. Consider:
> ```
> #define FOO 1
> 
> void g(int);
> void h(double);
> void i(const char *);
> 
> void f() {
>   g(FOO); // Probably don't want to suggest comments here
>   g((1)); // Probably do want to suggest comments here
>   h(1.0f); // Probably do want to suggest comments here
>   i(__FILE__); // Probably don't want to suggest comments here
> }
> ```
> I think this code should do two things: 1) check whether the location of the arg is a macro expansion (if so, return false), 2) do `Arg = Arg->IgnoreParenImpCasts();` at the start of the function and drop the `Arg->IgnoreImpCasts()` for the string and pointer literal cases. However, that may be a bridge too far, so you should add a test case like this:
> ```
> g(_Generic(0, int : 0)); // Definitely do not want to see comments here
> ```
> If it turns out the above case gives the comment suggestions, then I'd go with `IgnoreImpCasts()` rather than `IgnoreParenImpCasts()`.
I agree, tried all combinations but g((1); and g(_Generic(0, int : 0)); would otherwise get comments


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

https://reviews.llvm.org/D57674