[clang-tidy] Suggest including <cmath> if necessary in type-promotion-in-math-fn-check.

[alexfh]

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

187

s/D/Diag/

198

Remove all spaces inside the argument comment, it's more common that way and clang-format understand it better.

[alexfh]

LG with a couple of nits.

[jlebar]

Thank you for the review, @alexfh. I will commit with these changes.

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

198

Done, but FWIW it seems that clang-format will not insert a linebreak between the comment and the arg even with the spaces. It is much more common in llvm, though, as you say.

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

 //===--- 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"
+#include "../utils/IncludeInserter.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) {}
+  TypePromotionInMathFnCheck(StringRef Name, ClangTidyContext *Context);
+
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  std::unique_ptr<utils::IncludeInserter> IncludeInserter;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
 };
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_TYPE_PROMOTION_IN_MATH_FN_H

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

 //===--- 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 "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.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
 
+TypePromotionInMathFnCheck::TypePromotionInMathFnCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+          Options.get("IncludeStyle", "llvm"))) {}
+
+void TypePromotionInMathFnCheck::registerPPCallbacks(
+    CompilerInstance &Compiler) {
+  IncludeInserter = llvm::make_unique<utils::IncludeInserter>(
+      Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle);
+  Compiler.getPreprocessor().addPPCallbacks(
+      IncludeInserter->CreatePPCallbacks());
+}
+
+void TypePromotionInMathFnCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IncludeStyle",
+                utils::IncludeSorter::toString(IncludeStyle));
+}
+
 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) {
static llvm::StringSet<> Cpp11OnlyFns = {
       "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;
+  bool FnInCmath = false;
   if (getLangOpts().CPlusPlus &&
-      (!StdFnRequiresCpp11 || getLangOpts().CPlusPlus11))
+      (!StdFnRequiresCpp11 || getLangOpts().CPlusPlus11)) {
     NewFnName = ("std::" + OldFnName).str();
-  else
+    FnInCmath = true;
+  } else {
     NewFnName = (OldFnName + "f").str();
+  }
 
-  diag(Call->getExprLoc(), "call to '%0' promotes float to double")
+  auto D = 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.
+  // Suggest including <cmath> if the function we're suggesting is declared in
+  // <cmath> and it's not already included.  We never have to suggest including
+  // <math.h>, because the functions we're suggesting moving away from are all
+  // declared in <math.h>.
+  if (FnInCmath)
+    if (auto IncludeFixit = IncludeInserter->CreateIncludeInsertion(
+            Result.Context->getSourceManager().getFileID(Call->getLocStart()),
+            "cmath", /* IsAngled = */ true))
+      D << *IncludeFixit;
 }
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang

clang-tools-extra/test/clang-tidy/performance-type-promotion-in-math-fn.cpp

 // RUN: %check_clang_tidy %s performance-type-promotion-in-math-fn %t
 
+// CHECK-FIXES: #include <cmath>
+
 double acos(double);
 double acosh(double);
 double asin(double);
 double asinh(double);
 double atan2(double, double);
 double atan(double);
 double atanh(double);
 double cbrt(double);