aaron.ballman added a comment. Can you drop the file mode changes that are in this review?
================ Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:292-293 + // If the argument comments are missing for literals add them. + if (Comments.empty()) { + if (((isa<CXXBoolLiteralExpr>(Args[I]) && AddCommentsToBoolLiterals) || + (isa<IntegerLiteral>(Args[I]) && AddCommentsToIntegerLiterals) || ---------------- These can be combined into a single `if` statement. ================ Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:300-301 + diag(Args[I]->getBeginLoc(), + "argument comment missing for literal argument" + " %0") + << II ---------------- Why is this split into two lines? ================ Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:304 + << FixItHint::CreateInsertion(Args[I]->getBeginLoc(), ArgComment); + continue; + } ---------------- This `continue` can be dropped without changing the semantics, correct? ================ Comment at: clang-tidy/bugprone/ArgumentCommentCheck.h:44-46 + const bool AddCommentsToBoolLiterals; + const bool AddCommentsToIntegerLiterals; + const bool AddCommentsToFloatLiterals; ---------------- Why not character or string literals? What about `nullptr` literals or UDLs? ================ Comment at: docs/clang-tidy/checks/bugprone-argument-comment.rst:40 + + void foo(bool turn_key,bool press_button); + ---------------- Format the code examples from our style guide as well (same below). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57674/new/ https://reviews.llvm.org/D57674 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits