[clang-tools-extra] [clang-tidy] Fix AllowImplicitlyDeletedCopyOrMove missing-move false positive (PR #207586)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Zeyi Xu (zeyi2)

<details>
<summary>Changes</summary>

The documentation for `cppcoreguidelines-special-member-functions` says that `AllowImplicitlyDeletedCopyOrMove` suppresses diagnostics for classes which implicitly delete copy or move operations.

While in practice, the check only suppressed the implicitly deleted copy operations, the move operations are missing because the user-declared destructor prevents their implicit declaration.

This commit suppress that missing-move diagnostic when both copy operations are implicitly deleted and no move operation is user-declared.

Godbolt: https://clang-tidy.godbolt.org/z/h1nT84Tee
Closes https://github.com/llvm/llvm-project/issues/196615

---
Full diff: https://github.com/llvm/llvm-project/pull/207586.diff


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp (+10-1) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) 
- (added) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-implicit-delete.cpp (+31) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
index 1ef64b702cd66..6f2ab250e441b 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -224,10 +224,19 @@ void SpecialMemberFunctionsCheck::checkForMissingMembers(
                    SpecialMemberFunctionKind::CopyAssignment);
   }
 
+  const bool CopyImplicitlyDeleted =
+      HasImplicitDeletedMember(SpecialMemberFunctionKind::CopyConstructor) &&
+      HasImplicitDeletedMember(SpecialMemberFunctionKind::CopyAssignment);
+  const bool MoveMissing =
+      !HasMember(SpecialMemberFunctionKind::MoveConstructor) &&
+      !HasMember(SpecialMemberFunctionKind::MoveAssignment);
+
   if (RequireFive &&
       !(AllowMissingMoveFunctionsWhenCopyIsDeleted &&
         (IsDeleted(SpecialMemberFunctionKind::CopyConstructor) &&
-         IsDeleted(SpecialMemberFunctionKind::CopyAssignment)))) {
+         IsDeleted(SpecialMemberFunctionKind::CopyAssignment))) &&
+      !(AllowImplicitlyDeletedCopyOrMove && CopyImplicitlyDeleted &&
+        MoveMissing)) {
     assert(RequireThree);
     RequireMembers(SpecialMemberFunctionKind::MoveConstructor,
                    SpecialMemberFunctionKind::MoveAssignment);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2a980cc089a65..0f6699c5e2270 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -538,6 +538,12 @@ Changes in existing checks
   by fixing a false positive on implicitly generated functions such as
   inherited constructors.
 
+- Improved :doc:`cppcoreguidelines-special-member-functions
+  <clang-tidy/checks/cppcoreguidelines/special-member-functions>` check by
+  fixing a false positive with the `AllowImplicitlyDeletedCopyOrMove` option
+  for classes whose copy operations are implicitly deleted by a non-copyable
+  base class.
+
 - Improved :doc:`cppcoreguidelines-use-enum-class
   <clang-tidy/checks/cppcoreguidelines/use-enum-class>` check by adding the
   `IgnoreMacros` option. When enabled, unscoped ``enum`` declarations within
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-implicit-delete.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-implicit-delete.cpp
new file mode 100644
index 0000000000000..45137a30c807c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-implicit-delete.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- \
+// RUN:   -config="{CheckOptions: {cppcoreguidelines-special-member-functions.AllowImplicitlyDeletedCopyOrMove: true}}"
+
+struct Base {
+  Base(const Base &) = delete;
+  Base &operator=(const Base &) = delete;
+  Base(Base &&) = delete;
+  Base &operator=(Base &&) = delete;
+  ~Base();
+};
+
+struct A : Base {
+  ~A() {}
+};
+
+struct Member {
+  Base B;
+  ~Member() {}
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:8: warning: class 'B' defines a non-default destructor and a move constructor but does not define a move assignment operator
+struct B : Base {
+  ~B() {}
+  B(B &&);
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:8: warning: class 'C' defines a non-default destructor and a move assignment operator but does not define a move constructor
+struct C : Base {
+  ~C() {}
+  C &operator=(C &&);
+};

``````````

</details>


https://github.com/llvm/llvm-project/pull/207586