Skip to content

[clang-tidy] Fix false positives with template in misc-unconventional-assign-operator check #143292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

flovent
Copy link
Contributor

@flovent flovent commented Jun 8, 2025

Fix false positives when copy assignment operator function in a template class returns the result of another assignment to *this, this check doesn't consider this situation that there will be a BinaryOperator for assignment rather than CXXOperatorCallExpr since this's type is dependent.

Closes #143237.

@llvmbot
Copy link
Member

llvmbot commented Jun 8, 2025

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: None (flovent)

Changes

Fix false positives when copy assignment operator function in a template class returns the result of another assignment to *this, this check doesn't consider this situation that there will be a BinaryOperator for assignment rather than CXXOperatorCallExpr since this's type is dependent.

Closes #143237.


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp (+5-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp (+13)
diff --git a/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
index afc4897eeb2ae..3fdaf9239f6af 100644
--- a/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -66,8 +66,11 @@ void UnconventionalAssignOperatorCheck::registerMatchers(
                                 hasArgument(0, cxxThisExpr())),
             cxxOperatorCallExpr(
                 hasOverloadedOperatorName("="),
-                hasArgument(
-                    0, unaryOperator(hasOperatorName("*"),
+                hasArgument(0, unaryOperator(hasOperatorName("*"),
+                                             hasUnaryOperand(cxxThisExpr())))),
+            binaryOperator(
+                hasOperatorName("="),
+                hasLHS(unaryOperator(hasOperatorName("*"),
                                      hasUnaryOperand(cxxThisExpr())))))))));
   const auto IsGoodAssign = cxxMethodDecl(IsAssign, HasGoodReturnType);
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a275d2ccfa004..394e18cd58f84 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -208,6 +208,11 @@ Changes in existing checks
   <clang-tidy/checks/misc/unused-using-decls>` check by fixing false positives
   on ``operator""`` with template parameters.
 
+- Improved :doc:`misc-unconventional-assign-operator
+  <clang-tidy/checks/misc/misc-unconventional-assign-operator>` check by fixing
+  false positives when copy assignment operator function in a template class
+  returns the result of another assignment to ``*this``(``return *this=...``).
+
 - Improved :doc:`misc-use-internal-linkage
   <clang-tidy/checks/misc/use-internal-linkage>` check by fix false positives
   for function or variable in header file which contains macro expansion and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp
index 74a22a7c083f4..9384d88c38fd0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp
@@ -163,3 +163,16 @@ struct TemplateTypeAlias {
   Alias3<TypeAlias::Alias> &operator=(double) { return *this; }
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'TemplateTypeAlias&' [misc-unconventional-assign-operator]
 };
+
+namespace issue143237 {
+template<typename T>
+struct B {
+  explicit B(int) {
+  }
+
+  B& operator=(int n) {
+    // No warning
+    return *this = B(n);
+  }
+};
+}

Copy link

github-actions bot commented Jun 8, 2025

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@@ -208,6 +208,11 @@ Changes in existing checks
<clang-tidy/checks/misc/unused-using-decls>` check by fixing false positives
on ``operator""`` with template parameters.

- Improved :doc:`misc-unconventional-assign-operator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be before misc-unused-using-decls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG|clang-tidy] false positive of misc-unconventional-assign-operator & cppcoreguidelines-c-copy-assignment-signature
3 participants