fwolff created this revision. fwolff added reviewers: aaron.ballman, alexfh, whisperity, mizvekov. fwolff added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, jeroen.dobbelaere, rnkovacs, xazax.hun. fwolff requested review of this revision. Herald added a subscriber: cfe-commits.
clang-tidy currently reports false positives even for simple cases such as: struct S { using X = S; X &operator=(const X&) { return *this; } }; This is due to the fact that the `misc-unconventional-assign-operator` check fails to look at the //canonical// types. This patch fixes this behavior. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D114197 Files: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp 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,26 @@ // 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: + 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; } +}; 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,
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,26 @@ // 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: + 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; } +}; 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,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits