-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang-tidy] Add portability-avoid-platform-specific-fundamental-types #146970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 81 commits
3ef4feb
48598ef
524fdd8
d5fd2e7
25425cc
86787ec
c66668e
1b314bb
411d598
b427df7
5dd5c72
7c2031b
475f1b2
20e43d8
20e9b32
f49a289
143fcad
3c2ccd5
40b8be3
93c8489
91eb765
723a76e
a30bedf
76125bb
8258324
8dd606b
90c9887
c487bf3
09a762d
a53b5bc
b525a7a
de662e8
d0012d3
304b7b0
e66cf7b
c2f53d9
32343ae
d37015e
9d47d7a
1fed634
110bb52
7c0003c
b3a206f
1361674
4a1af0c
bdbccb3
8c05d1f
03f0dfe
ed7157d
d26bb9e
9634a74
89d4489
4669797
a595421
69e4f38
950513d
6693940
49fb728
082a5ad
88bbef0
485022d
9a01bc0
2db38a1
a5d4073
9864a96
09158b0
6c4803a
568990b
a71bb2e
d96e8d3
87fbe9d
e4f9a80
c868336
5dc7c47
2a6222d
10a3a2e
8c115ad
c7c28c9
2104952
c725301
57e3e39
466133f
8138945
b02eccc
388fe0c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,186 @@ | ||||||
//===----------------------------------------------------------------------===// | ||||||
// | ||||||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||||||
// See https://llvm.org/LICENSE.txt for license information. | ||||||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||||||
// | ||||||
//===----------------------------------------------------------------------===// | ||||||
|
||||||
#include "AvoidPlatformSpecificFundamentalTypesCheck.h" | ||||||
#include "clang/AST/ASTContext.h" | ||||||
#include "clang/AST/Type.h" | ||||||
#include "clang/ASTMatchers/ASTMatchFinder.h" | ||||||
#include "clang/ASTMatchers/ASTMatchers.h" | ||||||
#include "clang/Basic/TargetInfo.h" | ||||||
|
||||||
using namespace clang::ast_matchers; | ||||||
|
||||||
namespace { | ||||||
|
||||||
AST_MATCHER(clang::QualType, isBuiltinInt) { | ||||||
const auto *BT = Node->getAs<clang::BuiltinType>(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't particularly like that we need to cast to builtinType(
anyOf(isBuiltinInt, isBuiltinFloat, ...)
) |
||||||
assert(BT); | ||||||
|
||||||
// BT->isInteger() would detect char and bool | ||||||
switch (BT->getKind()) { | ||||||
case clang::BuiltinType::Short: | ||||||
case clang::BuiltinType::UShort: | ||||||
case clang::BuiltinType::Int: | ||||||
case clang::BuiltinType::UInt: | ||||||
case clang::BuiltinType::Long: | ||||||
case clang::BuiltinType::ULong: | ||||||
case clang::BuiltinType::LongLong: | ||||||
case clang::BuiltinType::ULongLong: | ||||||
return true; | ||||||
default: | ||||||
return false; | ||||||
} | ||||||
} | ||||||
|
||||||
AST_MATCHER(clang::QualType, isBuiltinFloat) { | ||||||
const auto *BT = Node->getAs<clang::BuiltinType>(); | ||||||
assert(BT); | ||||||
|
||||||
return BT->isFloatingPoint(); | ||||||
} | ||||||
|
||||||
} // namespace | ||||||
|
||||||
namespace clang::tidy::portability { | ||||||
|
||||||
AvoidPlatformSpecificFundamentalTypesCheck:: | ||||||
AvoidPlatformSpecificFundamentalTypesCheck(StringRef Name, | ||||||
ClangTidyContext *Context) | ||||||
: ClangTidyCheck(Name, Context), | ||||||
WarnOnFloats(Options.get("WarnOnFloats", true)), | ||||||
WarnOnInts(Options.get("WarnOnInts", true)), | ||||||
WarnOnChars(Options.get("WarnOnChars", true)), | ||||||
IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", | ||||||
utils::IncludeSorter::IS_LLVM), | ||||||
areDiagsSelfContained()) { | ||||||
if (!WarnOnFloats && !WarnOnInts && !WarnOnChars) | ||||||
this->configurationDiag( | ||||||
"The check 'portability-avoid-platform-specific-fundamental-types' " | ||||||
"will not perform any analysis because 'WarnOnFloats', 'WarnOnInts' " | ||||||
"and 'WarnOnChars' are all false."); | ||||||
} | ||||||
|
||||||
void AvoidPlatformSpecificFundamentalTypesCheck::registerPPCallbacks( | ||||||
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { | ||||||
IncludeInserter.registerPreprocessor(PP); | ||||||
} | ||||||
|
||||||
void AvoidPlatformSpecificFundamentalTypesCheck::storeOptions( | ||||||
ClangTidyOptions::OptionMap &Opts) { | ||||||
Options.store(Opts, "WarnOnFloats", WarnOnFloats); | ||||||
Options.store(Opts, "WarnOnInts", WarnOnInts); | ||||||
Options.store(Opts, "WarnOnChars", WarnOnChars); | ||||||
Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle()); | ||||||
} | ||||||
|
||||||
static std::optional<std::string> getFloatReplacement(const BuiltinType *BT, | ||||||
ASTContext &Context) { | ||||||
const TargetInfo &Target = Context.getTargetInfo(); | ||||||
|
||||||
auto GetReplacementType = [](unsigned Width) -> std::optional<std::string> { | ||||||
switch (Width) { | ||||||
// This is ambiguous by default since it could be bfloat16 or float16 | ||||||
case 16U: | ||||||
return std::nullopt; | ||||||
case 32U: | ||||||
return "float32_t"; | ||||||
case 64U: | ||||||
return "float64_t"; | ||||||
case 128U: | ||||||
return "float128_t"; | ||||||
default: | ||||||
return std::nullopt; | ||||||
} | ||||||
}; | ||||||
|
||||||
switch (BT->getKind()) { | ||||||
// Not an ambiguous type | ||||||
case BuiltinType::BFloat16: | ||||||
return "bfloat16_t"; | ||||||
case BuiltinType::Half: | ||||||
return GetReplacementType(Target.getHalfWidth()); | ||||||
case BuiltinType::Float: | ||||||
return GetReplacementType(Target.getFloatWidth()); | ||||||
case BuiltinType::Double: | ||||||
return GetReplacementType(Target.getDoubleWidth()); | ||||||
case BuiltinType::LongDouble: | ||||||
return GetReplacementType(Target.getLongDoubleWidth()); | ||||||
case BuiltinType::Float128: | ||||||
return "float128_t"; | ||||||
default: | ||||||
return std::nullopt; | ||||||
} | ||||||
} | ||||||
|
||||||
void AvoidPlatformSpecificFundamentalTypesCheck::registerMatchers( | ||||||
MatchFinder *Finder) { | ||||||
auto PlatformSpecificFundamentalType = qualType(allOf( | ||||||
builtinType(), anyOf(WarnOnInts ? isBuiltinInt() : unless(anything()), | ||||||
WarnOnFloats ? isBuiltinFloat() : unless(anything()), | ||||||
WarnOnChars ? isChar() : unless(anything()), | ||||||
WarnOnChars ? isWideChar() : unless(anything())))); | ||||||
|
||||||
if (!WarnOnInts && !WarnOnFloats && !WarnOnChars) | ||||||
return; | ||||||
jj-marr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
Finder->addMatcher(typeLoc(loc(PlatformSpecificFundamentalType)).bind("type"), | ||||||
this); | ||||||
} | ||||||
|
||||||
void AvoidPlatformSpecificFundamentalTypesCheck::check( | ||||||
const MatchFinder::MatchResult &Result) { | ||||||
const auto *TL = Result.Nodes.getNodeAs<TypeLoc>("type"); | ||||||
assert(TL); | ||||||
|
||||||
const SourceLocation Loc = TL->getBeginLoc(); | ||||||
const QualType QT = TL->getType(); | ||||||
const SourceRange TypeRange = TL->getSourceRange(); | ||||||
|
||||||
// Skip implicit type locations, such as literals | ||||||
if (!Loc.isValid() || !TypeRange.isValid()) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC there is |
||||||
return; | ||||||
|
||||||
const std::string TypeName = QT.getUnqualifiedType().getAsString(); | ||||||
|
||||||
const auto *BT = QT->getAs<BuiltinType>(); | ||||||
|
||||||
jj-marr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
assert(BT); | ||||||
if (BT->isFloatingPoint()) { | ||||||
const auto Replacement = getFloatReplacement(BT, *Result.Context); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
if (!Replacement.has_value()) { | ||||||
diag(Loc, "avoid using platform-dependent floating point type '%0'; " | ||||||
"consider using a type alias or fixed-width type instead") | ||||||
<< TypeName; | ||||||
return; | ||||||
} | ||||||
|
||||||
auto Diag = | ||||||
diag(Loc, "avoid using platform-dependent floating point type '%0'; " | ||||||
"consider using '%1' instead") | ||||||
<< TypeName << Replacement.value(); | ||||||
|
||||||
if (TypeRange.isValid()) | ||||||
Diag << FixItHint::CreateReplacement(TypeRange, Replacement.value()); | ||||||
|
||||||
if (auto IncludeFixit = IncludeInserter.createIncludeInsertion( | ||||||
Result.SourceManager->getFileID(Loc), "<stdfloat>")) { | ||||||
Diag << *IncludeFixit; | ||||||
} | ||||||
} else if (QT->isCharType() || QT->isWideCharType()) { | ||||||
diag(Loc, "avoid using platform-dependent character type '%0'; " | ||||||
"consider using 'char8_t' for text or 'std::byte' for bytes") | ||||||
<< TypeName; | ||||||
} else { | ||||||
diag(Loc, "avoid using platform-dependent fundamental integer type '%0'; " | ||||||
"consider using a type alias or fixed-width type instead") | ||||||
<< TypeName; | ||||||
} | ||||||
} | ||||||
|
||||||
} // namespace clang::tidy::portability |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//==------------------------------------------------------------------------==// | ||
|
||
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_AVOIDPLATFORMSPECIFICFUNDAMENTALTYPESCHECK_H | ||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_AVOIDPLATFORMSPECIFICFUNDAMENTALTYPESCHECK_H | ||
|
||
#include "../ClangTidyCheck.h" | ||
#include "../utils/IncludeInserter.h" | ||
|
||
namespace clang::tidy::portability { | ||
|
||
/// Detects fundamental types (int, short, long, long long, char, float, etc) | ||
/// and warns against their use due to platform-dependent behavior. | ||
/// | ||
/// For the user-facing documentation see: | ||
/// http://clang.llvm.org/extra/clang-tidy/checks/portability/avoid-platform-specific-fundamental-types.html | ||
jj-marr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
class AvoidPlatformSpecificFundamentalTypesCheck : public ClangTidyCheck { | ||
public: | ||
AvoidPlatformSpecificFundamentalTypesCheck(StringRef Name, | ||
ClangTidyContext *Context); | ||
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, | ||
Preprocessor *ModuleExpanderPP) override; | ||
void registerMatchers(ast_matchers::MatchFinder *Finder) override; | ||
void check(const ast_matchers::MatchFinder::MatchResult &Result) override; | ||
void storeOptions(ClangTidyOptions::OptionMap &Opts) override; | ||
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { | ||
return LangOpts.CPlusPlus11; | ||
} | ||
std::optional<TraversalKind> getCheckTraversalKind() const override { | ||
return TK_IgnoreUnlessSpelledInSource; | ||
} | ||
|
||
private: | ||
const bool WarnOnFloats; | ||
const bool WarnOnInts; | ||
const bool WarnOnChars; | ||
utils::IncludeInserter IncludeInserter; | ||
}; | ||
|
||
} // namespace clang::tidy::portability | ||
|
||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_AVOIDPLATFORMSPECIFICFUNDAMENTALTYPESCHECK_H |
Uh oh!
There was an error while loading. Please reload this page.