[PATCH] D27748: [clang-tidy] Suggest including if necessary in type-promotion-in-math-fn-check.
jlebar marked 2 inline comments as done. jlebar added a comment. Thank you for the review, @alexfh. I will commit with these changes. Comment at: clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp:198 +Result.Context->getSourceManager().getFileID(Call->getLocStart()), +"cmath", /* IsAngled = */ true)) + D << *IncludeFixit; alexfh wrote: > Remove all spaces inside the argument comment, it's more common that way and > clang-format understand it better. 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. https://reviews.llvm.org/D27748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27748: [clang-tidy] Suggest including if necessary in type-promotion-in-math-fn-check.
This revision was automatically updated to reflect the committed changes.
jlebar marked an inline comment as done.
Closed by commit rL289637: [clang-tidy] Suggest including if necessary
in type-promotion-in-math… (authored by jlebar).
Changed prior to commit:
https://reviews.llvm.org/D27748?vs=81347=81348#toc
Repository:
rL LLVM
https://reviews.llvm.org/D27748
Files:
clang-tools-extra/trunk/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
clang-tools-extra/trunk/clang-tidy/performance/TypePromotionInMathFnCheck.h
clang-tools-extra/trunk/test/clang-tidy/performance-type-promotion-in-math-fn.cpp
Index: clang-tools-extra/trunk/clang-tidy/performance/TypePromotionInMathFnCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/performance/TypePromotionInMathFnCheck.h
+++ clang-tools-extra/trunk/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 ) override;
+ void storeOptions(ClangTidyOptions::OptionMap ) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+ std::unique_ptr IncludeInserter;
+ const utils::IncludeSorter::IncludeStyle IncludeStyle;
};
} // namespace performance
Index: clang-tools-extra/trunk/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
+++ clang-tools-extra/trunk/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 ) {
+ IncludeInserter = llvm::make_unique(
+ Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle);
+ Compiler.getPreprocessor().addPPCallbacks(
+ IncludeInserter->CreatePPCallbacks());
+}
+
+void TypePromotionInMathFnCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ 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,29 @@
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);
-
- // FIXME: Perhaps we should suggest #include if we suggest a cmath
- // function and cmath is not already included.
+ auto Diag = diag(Call->getExprLoc(), "call to '%0' promotes float to double")
+ << OldFnName
+ << FixItHint::CreateReplacement(
+ Call->getCallee()->getSourceRange(), NewFnName);
+
+ // Suggest including if the function we're suggesting is declared in
+ // and it's not already included. We never have to suggest including
+ // , because the functions we're suggesting moving away from are all
+ // declared in .
+ if (FnInCmath)
+if (auto IncludeFixit = IncludeInserter->CreateIncludeInsertion(
+Result.Context->getSourceManager().getFileID(Call->getLocStart()),
+"cmath", /*IsAngled=*/true))
+ Diag << *IncludeFixit;
}
} // namespace
[PATCH] D27748: [clang-tidy] Suggest including if necessary in type-promotion-in-math-fn-check.
alexfh added a comment. LG with a couple of nits. https://reviews.llvm.org/D27748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27748: [clang-tidy] Suggest including if necessary in type-promotion-in-math-fn-check.
alexfh accepted this revision. alexfh added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp:187 - 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( s/D/Diag/ Comment at: clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp:198 +Result.Context->getSourceManager().getFileID(Call->getLocStart()), +"cmath", /* IsAngled = */ true)) + D << *IncludeFixit; Remove all spaces inside the argument comment, it's more common that way and clang-format understand it better. https://reviews.llvm.org/D27748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27748: [clang-tidy] Suggest including if necessary in type-promotion-in-math-fn-check.
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
+
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 ) override;
+ void storeOptions(ClangTidyOptions::OptionMap ) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+ std::unique_ptr 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 ) {
+ IncludeInserter = llvm::make_unique(
+ Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle);
+ Compiler.getPreprocessor().addPPCallbacks(
+ IncludeInserter->CreatePPCallbacks());
+}
+
+void TypePromotionInMathFnCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ 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 if we suggest a cmath
- // function and cmath is not already included.
+ // Suggest including if the function we're suggesting is declared in
+ // and it's not already included. We never have to suggest including
+ // , because the functions we're suggesting moving away from are all
+ // declared in .
+ if (FnInCmath)
+if (auto
