[PATCH] D27748: [clang-tidy] Suggest including if necessary in type-promotion-in-math-fn-check.

[Justin Lebar via Phabricator via cfe-commits]

jlebar created this revision.
jlebar added a reviewer: alexfh.
jlebar added a subscriber: cfe-commits.
Herald added a subscriber: JDevlieghere.

https://reviews.llvm.org/D27748

Files:
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.h
  clang-tools-extra/test/clang-tidy/performance-type-promotion-in-math-fn.cpp


Index: clang-tools-extra/test/clang-tidy/performance-type-promotion-in-math-fn.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/performance-type-promotion-in-math-fn.cpp
+++ clang-tools-extra/test/clang-tidy/performance-type-promotion-in-math-fn.cpp
@@ -1,5 +1,7 @@
 // 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);
Index: clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.h
+++ clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.h
@@ -11,6 +11,7 @@
 #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 {
@@ -27,10 +28,16 @@
 /// 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
Index: clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
@@ -10,6 +10,8 @@
 #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;
@@ -27,6 +29,26 @@
 }
 } // 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;
@@ -153,18 +175,28 @@
   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")
-      << OldFnName << FixItHint::CreateReplacement(
-                          Call->getCallee()->getSourceRange(), NewFnName);
+  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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits