Skip to content

Commit

Permalink
[clang-tidy] Improved cppcoreguidelines-narrowing-conversions.IgnoreC…
Browse files Browse the repository at this point in the history
…onversionFromTypes (#69242)

Extended IgnoreConversionFromTypes option to include
types without a declaration, such as built-in types.
  • Loading branch information
PiotrZSL committed Oct 26, 2023
1 parent af07d7b commit fd06155
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"

Expand All @@ -23,6 +24,26 @@ using namespace clang::ast_matchers;

namespace clang::tidy::cppcoreguidelines {

namespace {

AST_MATCHER_P(QualType, hasAnyType, std::vector<StringRef>, Names) {
if (Names.empty())
return false;

std::string Name = Node.getLocalUnqualifiedType().getAsString();
return llvm::any_of(Names, [&Name](StringRef Ref) { return Ref == Name; });
}

AST_MATCHER(FieldDecl, hasIntBitwidth) {
assert(Node.isBitField());
const ASTContext &Ctx = Node.getASTContext();
unsigned IntBitWidth = Ctx.getIntWidth(Ctx.IntTy);
unsigned CurrentBitWidth = Node.getBitWidthValue(Ctx);
return IntBitWidth == CurrentBitWidth;
}

} // namespace

NarrowingConversionsCheck::NarrowingConversionsCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
Expand Down Expand Up @@ -53,25 +74,22 @@ void NarrowingConversionsCheck::storeOptions(
Options.store(Opts, "PedanticMode", PedanticMode);
}

AST_MATCHER(FieldDecl, hasIntBitwidth) {
assert(Node.isBitField());
const ASTContext &Ctx = Node.getASTContext();
unsigned IntBitWidth = Ctx.getIntWidth(Ctx.IntTy);
unsigned CurrentBitWidth = Node.getBitWidthValue(Ctx);
return IntBitWidth == CurrentBitWidth;
}

void NarrowingConversionsCheck::registerMatchers(MatchFinder *Finder) {
// ceil() and floor() are guaranteed to return integers, even though the type
// is not integral.
const auto IsCeilFloorCallExpr = expr(callExpr(callee(functionDecl(
hasAnyName("::ceil", "::std::ceil", "::floor", "::std::floor")))));

std::vector<StringRef> IgnoreConversionFromTypesVec =
utils::options::parseStringList(IgnoreConversionFromTypes);

// We may want to exclude other types from the checks, such as `size_type`
// and `difference_type`. These are often used to count elements, represented
// in 64 bits and assigned to `int`. Rarely are people counting >2B elements.
const auto IsConversionFromIgnoredType = hasType(namedDecl(
hasAnyName(utils::options::parseStringList(IgnoreConversionFromTypes))));
const auto IsConversionFromIgnoredType =
anyOf(hasType(namedDecl(hasAnyName(IgnoreConversionFromTypesVec))),
allOf(unless(hasType(namedDecl())),
hasType(qualType(hasAnyType(IgnoreConversionFromTypesVec)))));

// `IsConversionFromIgnoredType` will ignore narrowing calls from those types,
// but not expressions that are promoted to an ignored type as a result of a
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ Changes in existing checks
coroutine functions and increase issue detection for cases involving type
aliases with references.

- Improved :doc:`cppcoreguidelines-narrowing-conversions
<clang-tidy/checks/cppcoreguidelines/narrowing-conversions>` check by
extending the `IgnoreConversionFromTypes` option to include types without a
declaration, such as built-in types.

- Improved :doc:`cppcoreguidelines-prefer-member-initializer
<clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check to
ignore delegate constructors.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
// RUN: cppcoreguidelines-narrowing-conversions %t -- \
// RUN: -config='{CheckOptions: { \
// RUN: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes: "global_size_t;nested_size_type" \
// RUN: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes: "global_size_t;nested_size_type;long" \
// RUN: }}'

// We use global_size_t instead of 'size_t' because windows predefines size_t.
Expand Down Expand Up @@ -72,3 +72,10 @@ void most_narrowing_is_not_ok() {
// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
// CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
}

void test_ignore_builtin_type_pr58809() {
long x = 123;
short y = x;
// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:13: warning: narrowing conversion from 'long' to signed type 'short' is implementation-defined [cppcoreguidelines-narrowing-conversions]
// CHECK-MESSAGES-NOT-IGNORED: :[[@LINE-2]]:13: warning: narrowing conversion from 'long' to signed type 'short' is implementation-defined [cppcoreguidelines-narrowing-conversions]
}

0 comments on commit fd06155

Please sign in to comment.