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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:228-236
+static bool isStringLiteral(const Expr *Arg) {
+  const auto *Cast = dyn_cast<ImplicitCastExpr>(Arg);
+  return Cast ? isa<StringLiteral>(Cast->getSubExpr()) : false;
+}
+
+static bool isNullPtrLiteral(const Expr *Arg) {
+  const auto *Cast = dyn_cast<ImplicitCastExpr>(Arg);
----------------
What's going on with these? Why not `return isa<Blah>(Arg->IgnoreImpCasts());` (at which point, no need for the separate functions).


================
Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:312-318
+        (((CommentBoolLiterals && isa<CXXBoolLiteralExpr>(Args[I])) ||
+          (CommentIntegerLiterals && isa<IntegerLiteral>(Args[I])) ||
+          (CommentFloatLiterals && isa<FloatingLiteral>(Args[I])) ||
+          (CommentUserDefinedLiterals && isa<UserDefinedLiteral>(Args[I])) ||
+          (CommentCharacterLiterals && isa<CharacterLiteral>(Args[I])) ||
+          (CommentStringLiterals && isStringLiteral(Args[I])) ||
+          (CommentNullPtrs && isNullPtrLiteral(Args[I]))))) {
----------------
This is a bit... much. I'm not certain what would be a better approach, but at the very least, factoring this out into a simple function call should make it a bit less... much. :-)


================
Comment at: clang-tidy/bugprone/ArgumentCommentCheck.h:43-50
   const bool StrictMode;
+  const bool CommentBoolLiterals;
+  const bool CommentIntegerLiterals;
+  const bool CommentFloatLiterals;
+  const bool CommentStringLiterals;
+  const bool CommentUserDefinedLiterals;
+  const bool CommentCharacterLiterals;
----------------
At this point, I think it would be better to turn these into: `const unsigned Blah : 1;`


================
Comment at: docs/clang-tidy/checks/bugprone-argument-comment.rst:40
+
+  void foo(bool turn_key,bool press_button);
+
----------------
aaron.ballman wrote:
> Format the code examples from our style guide as well (same below).
This still seems to be happening in the current revision?


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

https://reviews.llvm.org/D57674