[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc created this revision.
poelmanc added reviewers: malcolm.parsons, angelgarcia, aaron.ballman.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

`modernize-use-equals-default` replaces default constructors/destructors with `= default;`. When the optional semicolon after a member function <https://en.cppreference.com/w/cpp/language/class> is present, this results in two consecutive semicolons.

This small patch checks to see if the next non-comment token after the code to be replaced is a semicolon, and if so offers a replacement of `= default` rather than `= default;`.

This patch adds trailing comments and semicolons to about 5 existing tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D70144

Files:
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
@@ -7,7 +7,7 @@
   ~OL();
 };
 
-OL::OL() {}
+OL::OL() {};
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]
 // CHECK-FIXES: OL::OL() = default;
 OL::~OL() {}
@@ -17,9 +17,9 @@
 // Inline definitions.
 class IL {
 public:
-  IL() {}
+  IL() {} 	 ; // Note embedded tab on this line
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: IL() = default;
+  // CHECK-FIXES: IL() = default 	 ; // Note embedded tab on this line
   ~IL() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~IL() = default;
@@ -46,18 +46,20 @@
 // Default member initializer
 class DMI {
 public:
-  DMI() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: DMI() = default;
+  DMI() {} // Comment before semi-colon on next line
+  ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
+  // CHECK-FIXES: DMI() = default // Comment before semi-colon on next line
+  // CHECK-FIXES-NEXT:   ;
   int Field = 5;
 };
 
 // Class member
 class CM {
 public:
-  CM() {}
+  CM() {} /* Comments */ /* before */ /* semicolon */;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: CM() = default;
+  // CHECK-FIXES: CM() = default /* Comments */ /* before */ /* semicolon */;
   OL o;
 };
 
@@ -66,7 +68,7 @@
   Priv() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: Priv() = default;
-  ~Priv() {}
+  ~Priv() {};
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~Priv() = default;
 };
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
@@ -119,7 +119,7 @@
 struct BF {
   BF() = default;
   BF(const BF &Other) : Field1(Other.Field1), Field2(Other.Field2), Field3(Other.Field3),
-                        Field4(Other.Field4) {}
+                        Field4(Other.Field4) {};
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
   // CHECK-FIXES: BF(const BF &Other) {{$}}
   // CHECK-FIXES:                     = default;
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -302,9 +302,19 @@
   auto Diag = diag(Location, "use '= default' to define a trivial " +
                                  SpecialFunctionName);
 
-  if (ApplyFix)
-    Diag << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
+  if (ApplyFix) {
+    // Peek ahead to see if there's a semicolon after Body->getSourceRange()
+    Optional<Token> Token;
+    do {
+      Token = Lexer::findNextToken(
+          Body->getSourceRange().getEnd().getLocWithOffset(1),
+          Result.Context->getSourceManager(), Result.Context->getLangOpts());
+    } while (Token && Token->is(tok::comment));
+    StringRef Replacement =
+        Token && Token->is(tok::semi) ? "= default" : "= default;";
+    Diag << FixItHint::CreateReplacement(Body->getSourceRange(), Replacement)
          << RemoveInitializers;
+  }
 }
 
 } // namespace modernize


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70144.228950.patch
Type: text/x-patch
Size: 3815 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191112/4aea0136/attachment-0001.bin>