PiotrZSL created this revision. PiotrZSL added reviewers: njames93, carlosgalvezp, Izaron, fwolff. Herald added a subscriber: xazax.hun. Herald added a project: All. PiotrZSL requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.
Fixed issue with the comparison of UnaryExprOrTypeTraitExpr objects in which only the argument type was checked, without considering the kind of expression. This led to false positives when using sizeof(x) and alignof(x) expressions, as only the comparison x = x was performed without checking if sizeof was equal to alignof. Fixes: https://github.com/llvm/llvm-project/issues/63096 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D152713 Files: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp Index: clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp +++ clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp @@ -843,3 +843,15 @@ return 2; } + +namespace PR63096 { + +struct alignas(sizeof(int)) X { + int x; +}; + +static_assert(alignof(X) == sizeof(X)); +static_assert(sizeof(X) == sizeof(X)); +// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: both sides of operator are equivalent + +} Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -306,6 +306,10 @@ <clang-tidy/checks/misc/definitions-in-headers>` to avoid warning on declarations inside anonymous namespaces. +- Fixed false-positive in :doc:`misc-redundant-expression + <clang-tidy/checks/misc/redundant-expression>` check where expressions like + ``alignof`` or ``sizeof`` were incorrectly flagged as identical. + - Improved :doc:`misc-unused-parameters <clang-tidy/checks/misc/unused-parameters>` check with new `IgnoreVirtual` option to optionally ignore virtual methods. Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp @@ -144,8 +144,9 @@ const auto *RightUnaryExpr = cast<UnaryExprOrTypeTraitExpr>(Right); if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType()) - return LeftUnaryExpr->getArgumentType() == - RightUnaryExpr->getArgumentType(); + return LeftUnaryExpr->getKind() == RightUnaryExpr->getKind() && + LeftUnaryExpr->getArgumentType() == + RightUnaryExpr->getArgumentType(); if (!LeftUnaryExpr->isArgumentType() && !RightUnaryExpr->isArgumentType()) return areEquivalentExpr(LeftUnaryExpr->getArgumentExpr(), RightUnaryExpr->getArgumentExpr());
Index: clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp +++ clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp @@ -843,3 +843,15 @@ return 2; } + +namespace PR63096 { + +struct alignas(sizeof(int)) X { + int x; +}; + +static_assert(alignof(X) == sizeof(X)); +static_assert(sizeof(X) == sizeof(X)); +// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: both sides of operator are equivalent + +} Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -306,6 +306,10 @@ <clang-tidy/checks/misc/definitions-in-headers>` to avoid warning on declarations inside anonymous namespaces. +- Fixed false-positive in :doc:`misc-redundant-expression + <clang-tidy/checks/misc/redundant-expression>` check where expressions like + ``alignof`` or ``sizeof`` were incorrectly flagged as identical. + - Improved :doc:`misc-unused-parameters <clang-tidy/checks/misc/unused-parameters>` check with new `IgnoreVirtual` option to optionally ignore virtual methods. Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp @@ -144,8 +144,9 @@ const auto *RightUnaryExpr = cast<UnaryExprOrTypeTraitExpr>(Right); if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType()) - return LeftUnaryExpr->getArgumentType() == - RightUnaryExpr->getArgumentType(); + return LeftUnaryExpr->getKind() == RightUnaryExpr->getKind() && + LeftUnaryExpr->getArgumentType() == + RightUnaryExpr->getArgumentType(); if (!LeftUnaryExpr->isArgumentType() && !RightUnaryExpr->isArgumentType()) return areEquivalentExpr(LeftUnaryExpr->getArgumentExpr(), RightUnaryExpr->getArgumentExpr());
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits