[clang-tidy] Fix bugprone-argument-comment-check to correctly ignore implicit constructors.
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.
Reviewers: gribozavr
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D67744
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@372317 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/clang-tidy/bugprone/ArgumentCommentCheck.cpp b/clang-tidy/bugprone/ArgumentCommentCheck.cpp
index c690e79..fa1d234 100644
--- a/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ b/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -337,7 +337,7 @@
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;
diff --git a/test/clang-tidy/bugprone-argument-comment.cpp b/test/clang-tidy/bugprone-argument-comment.cpp
index 08f8717..8a4d9e5 100644
--- a/test/clang-tidy/bugprone-argument-comment.cpp
+++ b/test/clang-tidy/bugprone-argument-comment.cpp
@@ -63,6 +63,28 @@
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); }
