[ClangTidy] Add new performance-type-promotion-in-math-fn check.

Summary:
This checks for calls to double-precision math.h with single-precision
arguments.  For example, it suggests replacing ::sin(0.f) with
::sinf(0.f).

Subscribers: mgorny, cfe-commits

Differential Revision: https://reviews.llvm.org/D27284

llvm-svn: 289627

clang-tools-extra/clang-tidy/performance/CMakeLists.txt

@@ -6,6 +6,7 @@ add_clang_library(clangTidyPerformanceModule
   ImplicitCastInLoopCheck.cpp
   InefficientStringConcatenationCheck.cpp
   PerformanceTidyModule.cpp
+  TypePromotionInMathFnCheck.cpp
   UnnecessaryCopyInitialization.cpp
   UnnecessaryValueParamCheck.cpp
 

clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp

@@ -10,11 +10,11 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
-#include "InefficientStringConcatenationCheck.h"
-
 #include "FasterStringFindCheck.h"
 #include "ForRangeCopyCheck.h"
 #include "ImplicitCastInLoopCheck.h"
+#include "InefficientStringConcatenationCheck.h"
+#include "TypePromotionInMathFnCheck.h"
 #include "UnnecessaryCopyInitialization.h"
 #include "UnnecessaryValueParamCheck.h"
 
@@ -33,6 +33,8 @@ class PerformanceModule : public ClangTidyModule {
         "performance-implicit-cast-in-loop");
     CheckFactories.registerCheck<InefficientStringConcatenationCheck>(
         "performance-inefficient-string-concatenation");
+    CheckFactories.registerCheck<TypePromotionInMathFnCheck>(
+        "performance-type-promotion-in-math-fn");
     CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
         "performance-unnecessary-copy-initialization");
     CheckFactories.registerCheck<UnnecessaryValueParamCheck>(

clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp

@@ -0,0 +1,172 @@
+//===--- TypePromotionInMathFnCheck.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 "TypePromotionInMathFnCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringSet.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+namespace {
+AST_MATCHER_P(Type, isBuiltinType, BuiltinType::Kind, Kind) {
+  if (const auto *BT = dyn_cast<BuiltinType>(&Node)) {
+    return BT->getKind() == Kind;
+  }
+  return false;
+}
+} // anonymous namespace
+
+void TypePromotionInMathFnCheck::registerMatchers(MatchFinder *Finder) {
+  constexpr BuiltinType::Kind IntTy = BuiltinType::Int;
+  constexpr BuiltinType::Kind LongTy = BuiltinType::Long;
+  constexpr BuiltinType::Kind FloatTy = BuiltinType::Float;
+  constexpr BuiltinType::Kind DoubleTy = BuiltinType::Double;
+  constexpr BuiltinType::Kind LongDoubleTy = BuiltinType::LongDouble;
+
+  auto hasBuiltinTyParam = [](int Pos, BuiltinType::Kind Kind) {
+    return hasParameter(Pos, hasType(isBuiltinType(Kind)));
+  };
+  auto hasBuiltinTyArg = [](int Pos, BuiltinType::Kind Kind) {
+    return hasArgument(Pos, hasType(isBuiltinType(Kind)));
+  };
+
+  // Match calls to foo(double) with a float argument.
+  auto OneDoubleArgFns = hasAnyName(
+      "::acos", "::acosh", "::asin", "::asinh", "::atan", "::atanh", "::cbrt",
+      "::ceil", "::cos", "::cosh", "::erf", "::erfc", "::exp", "::exp2",
+      "::expm1", "::fabs", "::floor", "::ilogb", "::lgamma", "::llrint",
+      "::log", "::log10", "::log1p", "::log2", "::logb", "::lrint", "::modf",
+      "::nearbyint", "::rint", "::round", "::sin", "::sinh", "::sqrt", "::tan",
+      "::tanh", "::tgamma", "::trunc", "::llround", "::lround");
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(OneDoubleArgFns, parameterCountIs(1),
+                                   hasBuiltinTyParam(0, DoubleTy))),
+               hasBuiltinTyArg(0, FloatTy))
+          .bind("call"),
+      this);
+
+  // Match calls to foo(double, double) where both args are floats.
+  auto TwoDoubleArgFns = hasAnyName("::atan2", "::copysign", "::fdim", "::fmax",
+                                    "::fmin", "::fmod", "::hypot", "::ldexp",
+                                    "::nextafter", "::pow", "::remainder");
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(TwoDoubleArgFns, parameterCountIs(2),
+                                   hasBuiltinTyParam(0, DoubleTy),
+                                   hasBuiltinTyParam(1, DoubleTy))),
+               hasBuiltinTyArg(0, FloatTy), hasBuiltinTyArg(1, FloatTy))
+          .bind("call"),
+      this);
+
+  // Match calls to fma(double, double, double) where all args are floats.
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(hasName("::fma"), parameterCountIs(3),
+                                   hasBuiltinTyParam(0, DoubleTy),
+                                   hasBuiltinTyParam(1, DoubleTy),
+                                   hasBuiltinTyParam(2, DoubleTy))),
+               hasBuiltinTyArg(0, FloatTy), hasBuiltinTyArg(1, FloatTy),
+               hasBuiltinTyArg(2, FloatTy))
+          .bind("call"),
+      this);
+
+  // Match calls to frexp(double, int*) where the first arg is a float.
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(
+                   hasName("::frexp"), parameterCountIs(2),
+                   hasBuiltinTyParam(0, DoubleTy),
+                   hasParameter(1, parmVarDecl(hasType(pointerType(
+                                       pointee(isBuiltinType(IntTy)))))))),
+               hasBuiltinTyArg(0, FloatTy))
+          .bind("call"),
+      this);
+
+  // Match calls to nexttoward(double, long double) where the first arg is a
+  // float.
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(hasName("::nexttoward"), parameterCountIs(2),
+                                   hasBuiltinTyParam(0, DoubleTy),
+                                   hasBuiltinTyParam(1, LongDoubleTy))),
+               hasBuiltinTyArg(0, FloatTy))
+          .bind("call"),
+      this);
+
+  // Match calls to remquo(double, double, int*) where the first two args are
+  // floats.
+  Finder->addMatcher(
+      callExpr(
+          callee(functionDecl(
+              hasName("::remquo"), parameterCountIs(3),
+              hasBuiltinTyParam(0, DoubleTy), hasBuiltinTyParam(1, DoubleTy),
+              hasParameter(2, parmVarDecl(hasType(pointerType(
+                                  pointee(isBuiltinType(IntTy)))))))),
+          hasBuiltinTyArg(0, FloatTy), hasBuiltinTyArg(1, FloatTy))
+          .bind("call"),
+      this);
+
+  // Match calls to scalbln(double, long) where the first arg is a float.
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(hasName("::scalbln"), parameterCountIs(2),
+                                   hasBuiltinTyParam(0, DoubleTy),
+                                   hasBuiltinTyParam(1, LongTy))),
+               hasBuiltinTyArg(0, FloatTy))
+          .bind("call"),
+      this);
+
+  // Match calls to scalbn(double, int) where the first arg is a float.
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(hasName("::scalbn"), parameterCountIs(2),
+                                   hasBuiltinTyParam(0, DoubleTy),
+                                   hasBuiltinTyParam(1, IntTy))),
+               hasBuiltinTyArg(0, FloatTy))
+          .bind("call"),
+      this);
+
+  // modf(double, double*) is omitted because the second parameter forces the
+  // type -- there's no conversion from float* to double*.
+}
+
+void TypePromotionInMathFnCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");
+  assert(Call != nullptr);
+
+  StringRef OldFnName = Call->getDirectCallee()->getName();
+
+  // In C++ mode, we prefer std::foo to ::foof.  But some of these suggestions
+  // are only valid in C++11 and newer.
+  static llvm::StringSet<> Cpp11OnlyFns = {
+      "acosh",     "asinh",      "atanh",     "cbrt",   "copysign", "erf",
+      "erfc",      "exp2",       "expm1",     "fdim",   "fma",      "fmax",
+      "fmin",      "hypot",      "ilogb",     "lgamma", "llrint",   "llround",
+      "log1p",     "log2",       "logb",      "lrint",  "lround",   "nearbyint",
+      "nextafter", "nexttoward", "remainder", "remquo", "rint",     "round",
+      "scalbln",   "scalbn",     "tgamma",    "trunc"};
+  bool StdFnRequiresCpp11 = Cpp11OnlyFns.count(OldFnName);
+
+  std::string NewFnName;
+  if (getLangOpts().CPlusPlus &&
+      (!StdFnRequiresCpp11 || getLangOpts().CPlusPlus11))
+    NewFnName = ("std::" + OldFnName).str();
+  else
+    NewFnName = (OldFnName + "f").str();
+
+  diag(Call->getExprLoc(), "call to '%0' promotes float to double")
+      << OldFnName << FixItHint::CreateReplacement(
+                          Call->getCallee()->getSourceRange(), NewFnName);
+
+  // FIXME: Perhaps we should suggest #include <cmath> if we suggest a cmath
+  // function and cmath is not already included.
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.h

@@ -0,0 +1,40 @@
+//===--- TypePromotionInMathFnCheck.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_PERFORMANCE_TYPE_PROMOTION_IN_MATH_FN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_TYPE_PROMOTION_IN_MATH_FN_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// Finds calls to C math library functions with implicit float to double
+/// promotions.
+///
+/// For example, warns on ::sin(0.f), because this funciton's parameter is a
+/// double.  You probably meant to call std::sin(0.f) (in C++), or sinf(0.f) (in
+/// C).
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-type-promotion-in-math-fn.html
+class TypePromotionInMathFnCheck : public ClangTidyCheck {
+public:
+  TypePromotionInMathFnCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_TYPE_PROMOTION_IN_MATH_FN_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -124,6 +124,12 @@ Improvements to clang-tidy
   Warns about the performance overhead arising from concatenating strings using
   the ``operator+``, instead of ``operator+=``.
 
+- New `performance-type-promotion-in-math-fn
+  <http://clang.llvm.org/extra/clang-tidy/checks/performance-type-promotion-in-math-fn.html>`_ check
+
+  Replaces uses of C-style standard math functions with double parameters and float
+  arguments with an equivalent function that takes a float parameter.
+
 - `readability-container-size-empty
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-container-size-empty.html>`_ check
   supports arbitrary containers with with suitable ``empty()`` and ``size()``

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -123,6 +123,7 @@ Clang-Tidy Checks
    performance-for-range-copy
    performance-implicit-cast-in-loop
    performance-inefficient-string-concatenation
+   performance-type-promotion-in-math-fn
    performance-unnecessary-copy-initialization
    performance-unnecessary-value-param
    readability-avoid-const-params-in-decls

clang-tools-extra/docs/clang-tidy/checks/performance-type-promotion-in-math-fn.rst

@@ -0,0 +1,11 @@
+.. title:: clang-tidy - performance-type-promotion-in-math-fn
+
+performance-type-promotion-in-math-fn
+=====================================
+
+Finds calls to C math library functions (from ``math.h`` or, in C++, ``cmath``)
+with implicit ``float`` to ``double`` promotions.
+
+For example, warns on ``::sin(0.f)``, because this funciton's parameter is a
+double. You probably meant to call ``std::sin(0.f)`` (in C++), or ``sinf(0.f)``
+(in C).