[PATCH] D114197: [clang-tidy] Fix false positives involving type aliases in `misc-unconventional-assign-operator` check

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 391850.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114197/new/

https://reviews.llvm.org/D114197

Files:
  clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
@@ -127,3 +127,30 @@
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator=() should always return '*this'
   }
 };
+
+// Check that no false positives are issued when using type aliases
+struct TypeAlias {
+  using Alias = TypeAlias;
+  // This is correct and should not produce any warnings:
+  Alias &operator=(const Alias &) { return *this; }
+
+  using AliasRef = Alias &;
+  // So is this (assignments from other types are fine):
+  AliasRef operator=(int) { return *this; }
+};
+
+// Same check as above for a template class
+template <typename T>
+struct TemplateTypeAlias {
+  using Alias1 = TemplateTypeAlias &;
+  using Alias2 = TemplateTypeAlias const &;
+  Alias1 operator=(Alias2) { return *this; }
+
+  template <typename U>
+  using Alias3 = TemplateTypeAlias<U>;
+  Alias3<T> &operator=(int) { return *this; }
+
+  // Using a different type parameter in the return type should give a warning
+  Alias3<TypeAlias::Alias>& operator=(double) { return *this; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'TemplateTypeAlias&' [misc-unconventional-assign-operator]
+};
Index: clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
@@ -8,6 +8,8 @@
 types and definitions with good return type but wrong ``return`` statements.
 
   * The return type must be ``Class&``.
-  * Works with move-assign and assign by value.
+  * The assignment may be from the class type by value, const lvalue
+    reference, or non-const rvalue reference, or from a completely different
+    type (e.g. ``int``).
   * Private and deleted operators are ignored.
   * The operator must always return ``*this``.
Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -18,13 +18,14 @@
 
 void UnconventionalAssignOperatorCheck::registerMatchers(
     ast_matchers::MatchFinder *Finder) {
-  const auto HasGoodReturnType = cxxMethodDecl(returns(lValueReferenceType(
-      pointee(unless(isConstQualified()),
-              anyOf(autoType(), hasDeclaration(equalsBoundNode("class")))))));
+  const auto HasGoodReturnType =
+      cxxMethodDecl(returns(hasCanonicalType(lValueReferenceType(pointee(
+          unless(isConstQualified()),
+          anyOf(autoType(), hasDeclaration(equalsBoundNode("class"))))))));
 
-  const auto IsSelf = qualType(
+  const auto IsSelf = qualType(hasCanonicalType(
       anyOf(hasDeclaration(equalsBoundNode("class")),
-            referenceType(pointee(hasDeclaration(equalsBoundNode("class"))))));
+            referenceType(pointee(hasDeclaration(equalsBoundNode("class")))))));
   const auto IsAssign =
       cxxMethodDecl(unless(anyOf(isDeleted(), isPrivate(), isImplicit())),
                     hasName("operator="), ofClass(recordDecl().bind("class")))
@@ -37,9 +38,9 @@
       cxxMethodDecl(IsAssign, unless(HasGoodReturnType)).bind("ReturnType"),
       this);
 
-  const auto BadSelf = referenceType(
+  const auto BadSelf = qualType(hasCanonicalType(referenceType(
       anyOf(lValueReferenceType(pointee(unless(isConstQualified()))),
-            rValueReferenceType(pointee(isConstQualified()))));
+            rValueReferenceType(pointee(isConstQualified()))))));
 
   Finder->addMatcher(
       cxxMethodDecl(IsSelfAssign,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114197.391850.patch
Type: text/x-patch
Size: 4038 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211204/be6d92c9/attachment-0001.bin>