[clang-tidy] Fix bugprone-argument-comment-check to correctly ignore implicit constructors.

[ymandel]

Summary

After revision 370919, this check incorrectly flags certain cases of implicit
constructors. Specifically, if an argument is annotated with an
argument-comment and the argument expression triggers an implicit constructor,
then the argument comment is associated with argument of the implicit
constructor.

However, this only happens when the constructor has more than one argument.
This revision fixes the check for implicit constructors and adds a regression
test for this case.

Note: r370919 didn't cause this bug, it simply uncovered it by fixing another
bug that was masking the behavior.

[gribozavr]

Thanks for the fix!

clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp

if (const auto *Call = dyn_cast<CallExpr>(E)) {
     const FunctionDecl *Callee = Call->getDirectCallee();
     if (!Callee)
       return;
 
     checkCallArgs(Result.Context, Callee, Call->getCallee()->getEndLoc(),
                   llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
   } else {
     const auto *Construct = cast<CXXConstructExpr>(E);
-    if (Construct->getNumArgs() == 1 &&
+    if (Construct->getNumArgs() > 0 &&
         Construct->getArg(0)->getSourceRange() == Construct->getSourceRange()) {
       // Ignore implicit construction.
       return;
     }
     checkCallArgs(
         Result.Context, Construct->getConstructor(),
         Construct->getParenOrBraceRange().getBegin(),
         llvm::makeArrayRef(Construct->getArgs(), Construct->getNumArgs()));
   }
 }
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp

 // CHECK-NOTES: [[@LINE-3]]:15: note: 'aaa' declared here
 // CHECK-FIXES: void f2() { qqq(/*bbb=*/FALSE); }
 
 void f3(bool _with_underscores_);
 void ignores_underscores() {
   f3(/*With_Underscores=*/false);
 }
 
+namespace IgnoresImplicit {
+struct S {
+  S(int x);
+  int x;
+};
+
+struct T {
+  // Use two arguments (one defaulted) because simplistic check for implicit
+  // constructor looks for only one argument. We need to default the argument so
+  // that it will still be triggered implicitly.  This is not contrived -- it
+  // comes up in real code, for example std::set(std::initializer_list...).
+  T(S s, int y = 0);
+};
+
+void k(T arg1);
+
+void mynewtest() {
+  int foo = 3;
+  k(/*arg1=*/S(foo));
+}
+} // namespace IgnoresImplicit
+
 namespace ThisEditDistanceAboveThreshold {
 void f4(int xxx);
 void g() { f4(/*xyz=*/0); }
 // CHECK-NOTES: [[@LINE-1]]:15: warning: argument name 'xyz' in comment does not match parameter name 'xxx'
 // CHECK-NOTES: [[@LINE-3]]:13: note: 'xxx' declared here
 // CHECK-FIXES: void g() { f4(/*xyz=*/0); }
 }