[clang-tools-extra] r212942 - [clang-tidy] Add a checker for swapped arguments.
Benjamin Kramer
benny.kra at googlemail.com
Mon Jul 14 07:24:31 PDT 2014
Author: d0k
Date: Mon Jul 14 09:24:30 2014
New Revision: 212942
URL: http://llvm.org/viewvc/llvm-project?rev=212942&view=rev
Log:
[clang-tidy] Add a checker for swapped arguments.
This looks for swapped arguments by looking at implicit conversions of arguments
void Foo(int, double);
Foo(1.0, 3); // Most likely a bug
Added:
clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.h
clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=212942&r1=212941&r2=212942&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Mon Jul 14 09:24:30 2014
@@ -5,6 +5,7 @@ add_clang_library(clangTidyMiscModule
BoolPointerImplicitConversion.cpp
MiscTidyModule.cpp
RedundantSmartptrGet.cpp
+ SwappedArgumentsCheck.cpp
UseOverride.cpp
LINK_LIBS
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=212942&r1=212941&r2=212942&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Mon Jul 14 09:24:30 2014
@@ -13,6 +13,7 @@
#include "ArgumentCommentCheck.h"
#include "BoolPointerImplicitConversion.h"
#include "RedundantSmartptrGet.h"
+#include "SwappedArgumentsCheck.h"
#include "UseOverride.h"
namespace clang {
@@ -31,6 +32,9 @@ public:
"misc-redundant-smartptr-get",
new ClangTidyCheckFactory<RedundantSmartptrGet>());
CheckFactories.addCheckFactory(
+ "misc-swapped-arguments",
+ new ClangTidyCheckFactory<SwappedArgumentsCheck>());
+ CheckFactories.addCheckFactory(
"misc-use-override",
new ClangTidyCheckFactory<UseOverride>());
}
Added: clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp?rev=212942&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp Mon Jul 14 09:24:30 2014
@@ -0,0 +1,125 @@
+//===--- SwappedArgumentsCheck.cpp - clang-tidy ---------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SwappedArgumentsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void SwappedArgumentsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(callExpr().bind("call"), this);
+}
+
+/// \brief Look through lvalue to rvalue and nop casts. This filters out
+/// implicit conversions that have no effect on the input but block our view for
+/// other implicit casts.
+static const Expr *ignoreNoOpCasts(const Expr *E) {
+ if (auto *Cast = dyn_cast<CastExpr>(E))
+ if (Cast->getCastKind() == CK_LValueToRValue ||
+ Cast->getCastKind() == CK_NoOp)
+ return ignoreNoOpCasts(Cast->getSubExpr());
+ return E;
+}
+
+/// \brief Restrict the warning to implicit casts that are most likely
+/// accidental. User defined or integral conversions fit in this category,
+/// lvalue to rvalue or derived to base does not.
+static bool isImplicitCastCandidate(const CastExpr *Cast) {
+ return Cast->getCastKind() == CK_UserDefinedConversion ||
+ Cast->getCastKind() == CK_FloatingToBoolean ||
+ Cast->getCastKind() == CK_FloatingToIntegral ||
+ Cast->getCastKind() == CK_IntegralToBoolean ||
+ Cast->getCastKind() == CK_IntegralToFloating ||
+ Cast->getCastKind() == CK_MemberPointerToBoolean ||
+ Cast->getCastKind() == CK_PointerToBoolean;
+}
+
+/// \brief Get a StringRef representing a SourceRange.
+static StringRef getAsString(const MatchFinder::MatchResult &Result,
+ SourceRange R) {
+ const SourceManager &SM = *Result.SourceManager;
+ // Don't even try to resolve macro or include contraptions. Not worth emitting
+ // a fixit for.
+ if (R.getBegin().isMacroID() ||
+ !SM.isWrittenInSameFile(R.getBegin(), R.getEnd()))
+ return StringRef();
+
+ const char *Begin = SM.getCharacterData(R.getBegin());
+ const char *End = SM.getCharacterData(Lexer::getLocForEndOfToken(
+ R.getEnd(), 0, SM, Result.Context->getLangOpts()));
+
+ return StringRef(Begin, End - Begin);
+}
+
+void SwappedArgumentsCheck::check(const MatchFinder::MatchResult &Result) {
+ auto *Call = Result.Nodes.getStmtAs<CallExpr>("call");
+
+ llvm::SmallPtrSet<const Expr *, 4> UsedArgs;
+ for (unsigned I = 1, E = Call->getNumArgs(); I < E; ++I) {
+ const Expr *LHS = Call->getArg(I - 1);
+ const Expr *RHS = Call->getArg(I);
+
+ // Only need to check RHS, as LHS has already been covered. We don't want to
+ // emit two warnings for a single argument.
+ if (UsedArgs.count(RHS))
+ continue;
+
+ const auto *LHSCast = dyn_cast<ImplicitCastExpr>(ignoreNoOpCasts(LHS));
+ const auto *RHSCast = dyn_cast<ImplicitCastExpr>(ignoreNoOpCasts(RHS));
+
+ // Look if this is a potentially swapped argument pair. First look for
+ // implicit casts.
+ if (!LHSCast || !RHSCast || !isImplicitCastCandidate(LHSCast) ||
+ !isImplicitCastCandidate(RHSCast))
+ continue;
+
+ // If the types that go into the implicit casts match the types of the other
+ // argument in the declaration there is a high probability that the
+ // arguments were swapped.
+ // TODO: We could make use of the edit distance between the argument name
+ // and the name of the passed variable in addition to this type based
+ // heuristic.
+ const Expr *LHSFrom = ignoreNoOpCasts(LHSCast->getSubExpr());
+ const Expr *RHSFrom = ignoreNoOpCasts(RHSCast->getSubExpr());
+ if (LHS->getType() == RHS->getType() ||
+ LHS->getType() != RHSFrom->getType() ||
+ RHS->getType() != LHSFrom->getType())
+ continue;
+
+ // Emit a warning and fix-its that swap the arguments.
+ SourceRange LHSRange = LHS->getSourceRange(),
+ RHSRange = RHS->getSourceRange();
+ auto D =
+ diag(Call->getLocStart(), "argument with implicit conversion from %0 "
+ "to %1 followed by argument converted from "
+ "%2 to %3, potentially swapped arguments.")
+ << LHS->getType() << LHSFrom->getType() << RHS->getType()
+ << RHSFrom->getType() << LHSRange << RHSRange;
+
+ StringRef RHSString = getAsString(Result, RHSRange);
+ StringRef LHSString = getAsString(Result, LHSRange);
+ if (!LHSString.empty() && !RHSString.empty()) {
+ D << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(LHSRange), RHSString)
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(RHSRange), LHSString);
+ }
+
+ // Remember that we emitted a warning for this argument.
+ UsedArgs.insert(RHSCast);
+ }
+}
+
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.h?rev=212942&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.h Mon Jul 14 09:24:30 2014
@@ -0,0 +1,30 @@
+//===--- SwappedArgumentsCheck.h - clang-tidy -------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SWAPPED_ARGUMENTS_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SWAPPED_ARGUMENTS_CHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Finds potentially swapped arguments by looking at implicit
+/// conversions.
+class SwappedArgumentsCheck : public ClangTidyCheck {
+public:
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SWAPPED_ARGUMENTS_CHECK_H
+
Added: clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp?rev=212942&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp Mon Jul 14 09:24:30 2014
@@ -0,0 +1,44 @@
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-swapped-arguments %t
+// REQUIRES: shell
+
+void F(int, double);
+
+int SomeFunction();
+
+template <typename T, typename U>
+void G(T a, U b) {
+ F(a, b); // no-warning
+ F(2.0, 4);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: argument with implicit conversion from 'int' to 'double' followed by argument converted from 'double' to 'int', potentially swapped arguments.
+// CHECK-FIXES: F(4, 2.0)
+}
+
+void foo() {
+ F(1.0, 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: argument with implicit conversion from 'int' to 'double' followed by argument converted from 'double' to 'int', potentially swapped arguments.
+// CHECK-FIXES: F(3, 1.0)
+
+#define M(x, y) x##y()
+
+ double b = 1.0;
+ F(b, M(Some, Function));
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: argument with implicit conversion from 'int' to 'double' followed by argument converted from 'double' to 'int', potentially swapped arguments.
+// In macro, don't emit fixits.
+// CHECK-FIXES: F(b, M(Some, Function));
+
+#define N F(b, SomeFunction())
+
+ N;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: argument with implicit conversion from 'int' to 'double' followed by argument converted from 'double' to 'int', potentially swapped arguments.
+// In macro, don't emit fixits.
+// CHECK-FIXES: #define N F(b, SomeFunction())
+
+ G(b, 3);
+ G(3, 1.0);
+ G(0, 0);
+
+ F(1.0, 1.0); // no-warning
+ F(3, 1.0); // no-warning
+ F(true, false); // no-warning
+ F(0, 'c'); // no-warning
+}
More information about the cfe-commits
mailing list