[clang-tools-extra] [clang-tidy] Improved cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes (PR #69242)

via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 16 13:32:48 PDT 2023


https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/69242

>From 56bf257baff400189651c71a1639fbad8faa3a19 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Mon, 16 Oct 2023 19:43:21 +0000
Subject: [PATCH 1/2] [clang-tidy] Improved
 cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes

Extended IgnoreConversionFromTypes option to include
types without a declaration, such as built-in types.
---
 .../NarrowingConversionsCheck.cpp             | 38 ++++++++++++++-----
 clang-tools-extra/docs/ReleaseNotes.rst       |  5 +++
 ...sions-ignoreconversionfromtypes-option.cpp |  9 ++++-
 3 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
index 1b858db511f50a2..45fef9471d5211e 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -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"
 
@@ -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),
@@ -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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index af164d0462d52c1..5ad12aadd5b55b3 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -217,6 +217,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.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
index ab9aabf44ff68c5..fbd38e4dc98d8bb 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
@@ -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.
@@ -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;
+  int y = x;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:11: warning: narrowing conversion from 'long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-NOT-IGNORED: :[[@LINE-2]]:11: warning: narrowing conversion from 'long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}

>From b9a8ca5b88e4c4adcfb1965534375bfde16cb862 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Mon, 16 Oct 2023 20:32:23 +0000
Subject: [PATCH 2/2] [clang-tidy] Fixing tests on 32 bit platforms

---
 ...rrowing-conversions-ignoreconversionfromtypes-option.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
index fbd38e4dc98d8bb..91e908f535a0d40 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
@@ -75,7 +75,7 @@ void most_narrowing_is_not_ok() {
 
 void test_ignore_builtin_type_pr58809() {
   long x = 123;
-  int y = x;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:11: warning: narrowing conversion from 'long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
-  // CHECK-MESSAGES-NOT-IGNORED: :[[@LINE-2]]:11: warning: narrowing conversion from 'long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  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]
 }



More information about the cfe-commits mailing list