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

Yitzhak Mandelbaum via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 19 06:12:05 PDT 2019


Author: ymandel
Date: Thu Sep 19 06:12:05 2019
New Revision: 372317

URL: http://llvm.org/viewvc/llvm-project?rev=372317&view=rev
Log:
[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

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

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp?rev=372317&r1=372316&r2=372317&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp Thu Sep 19 06:12:05 2019
@@ -337,7 +337,7 @@ void ArgumentCommentCheck::check(const M
                   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;

Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp?rev=372317&r1=372316&r2=372317&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp Thu Sep 19 06:12:05 2019
@@ -63,6 +63,28 @@ 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); }




More information about the cfe-commits mailing list