This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG416e689ecda6: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments… (authored by whisperity).
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120555/new/ https://reviews.llvm.org/D120555 Files: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp Index: clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp +++ clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp @@ -485,3 +485,32 @@ return 0; } + +namespace Issue_54074 { + +class T {}; +using OperatorTy = int(const T &, const T &); +int operator-(const T &, const T &); + +template <typename U> +struct Wrap { + Wrap(U); +}; + +template <typename V> +void wrapTaker(V, Wrap<OperatorTy>); + +template <typename V> +void wrapTaker(V aaaaa, V bbbbb, Wrap<OperatorTy>); + +void test() { + wrapTaker(0, operator-); + // NO-WARN. No crash! + + int aaaaa = 4, bbbbb = 8; + wrapTaker(bbbbb, aaaaa, operator-); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'bbbbb' (passed to 'aaaaa') looks like it might be swapped with the 2nd, 'aaaaa' (passed to 'bbbbb') + // CHECK-MESSAGES: :[[@LINE-9]]:6: note: in the call to 'wrapTaker<int>', declared here +} + +} // namespace Issue_54074 Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -112,6 +112,9 @@ - Fixed a false positive in :doc:`readability-non-const-parameter <clang-tidy/checks/readability-non-const-parameter>` when the parameter is referenced by an lvalue +- Fixed a crash in :doc:`readability-suspicious-call-argument + <clang-tidy/checks/readability-suspicious-call-argument>` related to passing + arguments that refer to program elements without a trivial identifier. Removed checks ^^^^^^^^^^^^^^ Index: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp +++ clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp @@ -711,23 +711,28 @@ for (std::size_t I = InitialArgIndex, J = MatchedCallExpr->getNumArgs(); I < J; ++I) { + assert(ArgTypes.size() == I - InitialArgIndex && + ArgNames.size() == ArgTypes.size() && + "Every iteration must put an element into the vectors!"); + if (const auto *ArgExpr = dyn_cast<DeclRefExpr>( MatchedCallExpr->getArg(I)->IgnoreUnlessSpelledInSource())) { if (const auto *Var = dyn_cast<VarDecl>(ArgExpr->getDecl())) { ArgTypes.push_back(Var->getType()); ArgNames.push_back(Var->getName()); - } else if (const auto *FCall = - dyn_cast<FunctionDecl>(ArgExpr->getDecl())) { - ArgTypes.push_back(FCall->getType()); - ArgNames.push_back(FCall->getName()); - } else { - ArgTypes.push_back(QualType()); - ArgNames.push_back(StringRef()); + continue; + } + if (const auto *FCall = dyn_cast<FunctionDecl>(ArgExpr->getDecl())) { + if (FCall->getNameInfo().getName().isIdentifier()) { + ArgTypes.push_back(FCall->getType()); + ArgNames.push_back(FCall->getName()); + continue; + } } - } else { - ArgTypes.push_back(QualType()); - ArgNames.push_back(StringRef()); } + + ArgTypes.push_back(QualType()); + ArgNames.push_back(StringRef()); } }
Index: clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp +++ clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp @@ -485,3 +485,32 @@ return 0; } + +namespace Issue_54074 { + +class T {}; +using OperatorTy = int(const T &, const T &); +int operator-(const T &, const T &); + +template <typename U> +struct Wrap { + Wrap(U); +}; + +template <typename V> +void wrapTaker(V, Wrap<OperatorTy>); + +template <typename V> +void wrapTaker(V aaaaa, V bbbbb, Wrap<OperatorTy>); + +void test() { + wrapTaker(0, operator-); + // NO-WARN. No crash! + + int aaaaa = 4, bbbbb = 8; + wrapTaker(bbbbb, aaaaa, operator-); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'bbbbb' (passed to 'aaaaa') looks like it might be swapped with the 2nd, 'aaaaa' (passed to 'bbbbb') + // CHECK-MESSAGES: :[[@LINE-9]]:6: note: in the call to 'wrapTaker<int>', declared here +} + +} // namespace Issue_54074 Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -112,6 +112,9 @@ - Fixed a false positive in :doc:`readability-non-const-parameter <clang-tidy/checks/readability-non-const-parameter>` when the parameter is referenced by an lvalue +- Fixed a crash in :doc:`readability-suspicious-call-argument + <clang-tidy/checks/readability-suspicious-call-argument>` related to passing + arguments that refer to program elements without a trivial identifier. Removed checks ^^^^^^^^^^^^^^ Index: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp +++ clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp @@ -711,23 +711,28 @@ for (std::size_t I = InitialArgIndex, J = MatchedCallExpr->getNumArgs(); I < J; ++I) { + assert(ArgTypes.size() == I - InitialArgIndex && + ArgNames.size() == ArgTypes.size() && + "Every iteration must put an element into the vectors!"); + if (const auto *ArgExpr = dyn_cast<DeclRefExpr>( MatchedCallExpr->getArg(I)->IgnoreUnlessSpelledInSource())) { if (const auto *Var = dyn_cast<VarDecl>(ArgExpr->getDecl())) { ArgTypes.push_back(Var->getType()); ArgNames.push_back(Var->getName()); - } else if (const auto *FCall = - dyn_cast<FunctionDecl>(ArgExpr->getDecl())) { - ArgTypes.push_back(FCall->getType()); - ArgNames.push_back(FCall->getName()); - } else { - ArgTypes.push_back(QualType()); - ArgNames.push_back(StringRef()); + continue; + } + if (const auto *FCall = dyn_cast<FunctionDecl>(ArgExpr->getDecl())) { + if (FCall->getNameInfo().getName().isIdentifier()) { + ArgTypes.push_back(FCall->getType()); + ArgNames.push_back(FCall->getName()); + continue; + } } - } else { - ArgTypes.push_back(QualType()); - ArgNames.push_back(StringRef()); } + + ArgTypes.push_back(QualType()); + ArgNames.push_back(StringRef()); } }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits