[clang-tidy] Add portability-avoid-platform-specific-fundamental-types #146970
+870
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang-tidy Author: JJ Marr (jj-marr) ChangesThe fundamental types are widely recognized to be flawed because they're of implementation-defined sizes without a common purpose. For example, To address this, C/C++ introduced the TODO:
Patch is 22.04 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146970.diff 7 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.cpp
new file mode 100644
index 0000000000000..b5e2484e6396a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.cpp
@@ -0,0 +1,181 @@
+//===--- AvoidFundamentalIntegerTypesCheck.cpp - clang-tidy ---------------===//
+//
+// 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 "AvoidFundamentalIntegerTypesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+namespace {
+
+AST_MATCHER(clang::TypeLoc, hasValidBeginLoc) {
+ return Node.getBeginLoc().isValid();
+}
+
+AST_MATCHER_P(clang::TypeLoc, hasType,
+ clang::ast_matchers::internal::Matcher<clang::Type>,
+ InnerMatcher) {
+ const clang::Type *TypeNode = Node.getTypePtr();
+ return TypeNode != nullptr &&
+ InnerMatcher.matches(*TypeNode, Finder, Builder);
+}
+
+} // namespace
+
+AvoidFundamentalIntegerTypesCheck::AvoidFundamentalIntegerTypesCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+bool AvoidFundamentalIntegerTypesCheck::isFundamentalIntegerType(
+ const Type *T) const {
+ if (!T->isBuiltinType())
+ return false;
+
+ const auto *BT = T->getAs<BuiltinType>();
+ if (!BT)
+ return false;
+
+ switch (BT->getKind()) {
+ case BuiltinType::Int:
+ case BuiltinType::UInt:
+ case BuiltinType::Short:
+ case BuiltinType::UShort:
+ case BuiltinType::Long:
+ case BuiltinType::ULong:
+ case BuiltinType::LongLong:
+ case BuiltinType::ULongLong:
+ return true;
+ default:
+ return false;
+ }
+}
+
+bool AvoidFundamentalIntegerTypesCheck::isSemanticType(const Type *T) const {
+ if (!T->isBuiltinType())
+ return false;
+
+ const auto *BT = T->getAs<BuiltinType>();
+ if (!BT)
+ return false;
+
+ switch (BT->getKind()) {
+ case BuiltinType::Bool:
+ case BuiltinType::Char_S:
+ case BuiltinType::Char_U:
+ case BuiltinType::SChar:
+ case BuiltinType::UChar:
+ case BuiltinType::WChar_S:
+ case BuiltinType::WChar_U:
+ case BuiltinType::Char8:
+ case BuiltinType::Char16:
+ case BuiltinType::Char32:
+ return true;
+ default:
+ return false;
+ }
+}
+
+void AvoidFundamentalIntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
+ // Match variable declarations with fundamental integer types
+ Finder->addMatcher(
+ varDecl().bind("var_decl"),
+ this);
+
+ // Match function declarations with fundamental integer return types
+ Finder->addMatcher(
+ functionDecl().bind("func_decl"),
+ this);
+
+ // Match function parameters with fundamental integer types
+ Finder->addMatcher(
+ parmVarDecl().bind("param_decl"),
+ this);
+
+ // Match field declarations with fundamental integer types
+ Finder->addMatcher(
+ fieldDecl().bind("field_decl"),
+ this);
+
+ // Match typedef declarations to check their underlying types
+ Finder->addMatcher(
+ typedefDecl().bind("typedef_decl"),
+ this);
+
+ Finder->addMatcher(
+ typeAliasDecl().bind("alias_decl"),
+ this);
+}
+
+void AvoidFundamentalIntegerTypesCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ SourceLocation Loc;
+ QualType QT;
+ std::string DeclType;
+
+ if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("var_decl")) {
+ Loc = VD->getLocation();
+ QT = VD->getType();
+ DeclType = "variable";
+ } else if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("func_decl")) {
+ Loc = FD->getLocation();
+ QT = FD->getReturnType();
+ DeclType = "function return type";
+ } else if (const auto *PD = Result.Nodes.getNodeAs<ParmVarDecl>("param_decl")) {
+ Loc = PD->getLocation();
+ QT = PD->getType();
+ DeclType = "function parameter";
+ } else if (const auto *FD = Result.Nodes.getNodeAs<FieldDecl>("field_decl")) {
+ Loc = FD->getLocation();
+ QT = FD->getType();
+ DeclType = "field";
+ } else if (const auto *TD = Result.Nodes.getNodeAs<TypedefDecl>("typedef_decl")) {
+ Loc = TD->getLocation();
+ QT = TD->getUnderlyingType();
+ DeclType = "typedef";
+ } else if (const auto *AD = Result.Nodes.getNodeAs<TypeAliasDecl>("alias_decl")) {
+ Loc = AD->getLocation();
+ QT = AD->getUnderlyingType();
+ DeclType = "type alias";
+ } else {
+ return;
+ }
+
+ if (Loc.isInvalid() || QT.isNull())
+ return;
+
+ // Check if the type is already a typedef - if so, don't warn
+ // since the user is already using a typedef (which is what we want)
+ if (QT->getAs<TypedefType>()) {
+ return;
+ }
+
+ const Type *T = QT.getCanonicalType().getTypePtr();
+ if (!T)
+ return;
+
+ // Skip if not a fundamental integer type
+ if (!isFundamentalIntegerType(T))
+ return;
+
+ // Skip semantic types
+ if (isSemanticType(T))
+ return;
+
+ // Get the type name for the diagnostic
+ std::string TypeName = QT.getAsString();
+
+ diag(Loc, "avoid using platform-dependent fundamental integer type '%0'; "
+ "consider using a typedef or fixed-width type instead")
+ << TypeName;
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.h b/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.h
new file mode 100644
index 0000000000000..c1e4fb4748e5d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.h
@@ -0,0 +1,44 @@
+//===--- AvoidFundamentalIntegerTypesCheck.h - clang-tidy -------*- C++ -*-===//
+//
+// 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_MODERNIZE_AVOIDFUNDAMENTALINTEGERTYPESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDFUNDAMENTALINTEGERTYPESCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::modernize {
+
+/// Find fundamental integer types and recommend using typedefs or fixed-width types.
+///
+/// Detects fundamental integer types (int, short, long, long long, and their
+/// unsigned variants) and warns against their use due to platform-dependent
+/// behavior. Excludes semantic types like char, bool, wchar_t, char16_t,
+/// char32_t, size_t, and ptrdiff_t.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/avoid-fundamental-integer-types.html
+class AvoidFundamentalIntegerTypesCheck : public ClangTidyCheck {
+public:
+ AvoidFundamentalIntegerTypesCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus11;
+ }
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+
+private:
+ bool isFundamentalIntegerType(const Type *T) const;
+ bool isSemanticType(const Type *T) const;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDFUNDAMENTALINTEGERTYPESCHECK_H
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 619a27b2f9bb6..deb37c1ad9fb3 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyModernizeModule STATIC
AvoidBindCheck.cpp
AvoidCArraysCheck.cpp
+ AvoidFundamentalIntegerTypesCheck.cpp
ConcatNestedNamespacesCheck.cpp
DeprecatedHeadersCheck.cpp
DeprecatedIosBaseAliasesCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index fdf38bc4b6308..a42d55b26a311 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "AvoidBindCheck.h"
#include "AvoidCArraysCheck.h"
+#include "AvoidFundamentalIntegerTypesCheck.h"
#include "ConcatNestedNamespacesCheck.h"
#include "DeprecatedHeadersCheck.h"
#include "DeprecatedIosBaseAliasesCheck.h"
@@ -63,6 +64,8 @@ class ModernizeModule : public ClangTidyModule {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<AvoidBindCheck>("modernize-avoid-bind");
CheckFactories.registerCheck<AvoidCArraysCheck>("modernize-avoid-c-arrays");
+ CheckFactories.registerCheck<AvoidFundamentalIntegerTypesCheck>(
+ "modernize-avoid-fundamental-integer-types");
CheckFactories.registerCheck<ConcatNestedNamespacesCheck>(
"modernize-concat-nested-namespaces");
CheckFactories.registerCheck<DeprecatedHeadersCheck>(
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5098582d0c42b..33c2911fd9ae5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -280,6 +280,7 @@ Clang-Tidy Checks
:doc:`misc-use-internal-linkage <misc/use-internal-linkage>`, "Yes"
:doc:`modernize-avoid-bind <modernize/avoid-bind>`, "Yes"
:doc:`modernize-avoid-c-arrays <modernize/avoid-c-arrays>`,
+ :doc:`modernize-avoid-fundamental-integer-types <modernize/avoid-fundamental-integer-types>`,
:doc:`modernize-concat-nested-namespaces <modernize/concat-nested-namespaces>`, "Yes"
:doc:`modernize-deprecated-headers <modernize/deprecated-headers>`, "Yes"
:doc:`modernize-deprecated-ios-base-aliases <modernize/deprecated-ios-base-aliases>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-fundamental-integer-types.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-fundamental-integer-types.rst
new file mode 100644
index 0000000000000..df511e084701f
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-fundamental-integer-types.rst
@@ -0,0 +1,111 @@
+.. title:: clang-tidy - modernize-avoid-fundamental-integer-types
+
+modernize-avoid-fundamental-integer-types
+==========================================
+
+Finds fundamental integer types and recommends using typedefs or fixed-width types instead.
+
+This check detects fundamental integer types (``int``, ``short``, ``long``, ``long long``, and their
+``unsigned`` variants) and warns against their use due to non-standard platform-dependent behavior.
+For example, ``long`` is 64 bits on Linux but 32 bits on Windows. There is no standard rationale or
+intent for the sizes of these types.
+
+Instead of fundamental types, use fixed-width types such as ``int32_t`` or implementation-defined
+types with standard semantics, e.g. ``int_fast32_t`` for the fastest integer type greater than or
+equal to 32 bits.
+
+Examples
+--------
+
+.. code-block:: c++
+
+ // Bad: platform-dependent fundamental types
+ int global_int = 42;
+ short global_short = 10;
+ long global_long = 100L;
+ unsigned long global_unsigned_long = 100UL;
+
+ void function_with_int_param(int param) {
+ // ...
+ }
+
+ int function_returning_int() {
+ return 42;
+ }
+
+ struct MyStruct {
+ int member_int;
+ long member_long;
+ };
+
+.. code-block:: c++
+
+ // Good: use fixed-width types or typedefs
+ #include <cstdint>
+
+ int32_t global_int32 = 42;
+ int16_t global_int16 = 10;
+ int64_t global_int64 = 100L;
+ uint64_t global_uint64 = 100UL;
+
+ void function_with_int32_param(int32_t param) {
+ // ...
+ }
+
+ int32_t function_returning_int32() {
+ return 42;
+ }
+
+ struct MyStruct {
+ int32_t member_int32;
+ int64_t member_int64;
+ };
+
+The check will also warn about typedef declarations that use fundamental types as their underlying type:
+
+.. code-block:: c++
+
+ // Bad: typedef using fundamental type
+ typedef long long MyLongType;
+ using MyIntType = int;
+
+.. code-block:: c++
+
+ // Good: use descriptive names or fixed-width types
+ typedef int64_t TimestampType;
+ using CounterType = uint32_t;
+
+Rationale
+---------
+
+Fundamental integer types have platform-dependent sizes and behavior:
+
+- ``int`` is typically 32 bits on modern platforms but is only guaranteed to be 16 bits by the spec
+- ``long int`` is 32 bits on Windows but 64 bits on most Unix systems
+
+The C++ specification does not define these types beyond their minimum sizes. That means they can
+communicate intent in non-standard ways and are often needlessly incompatible. For example, ``int``
+was traditionally the word size of a given processor in 16-bit and 32-bit computing and was a
+reasonable default for performance. This is no longer true on modern 64-bit computers, but the size
+of ``int`` remains fixed at 32 bits for backwards compatibility with code that relied on a 32-bit
+implementation of ``int``.
+
+If code is explicitly relying on the size of an ``int`` being 32 bits, it is better to say so in
+the typename with ``int32_t``. Otherwise, use an appropriate implementation-defined type that
+communicates your intent.
+
+Types Not Flagged
+-----------------
+
+The following types are intentionally not flagged:
+
+- ``char``, ``signed char``, ``unsigned char`` (character types)
+- ``bool`` (boolean type)
+- Standard library typedefs like ``size_t``, ``ptrdiff_t``, or ``uint32_t``.
+- Already typedef'd types, though the check will flag the typedef itself
+
+``char`` is excluded because it is implementation-defined to always be 1 byte, regardless of the
+platform's definition of a byte.
+
+``bool`` is excluded because it can only be true or false, and is not vulnerable to overflow or
+narrowing issues that occur as a result of using implementation-defined types.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-fundamental-integer-types.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-fundamental-integer-types.cpp
new file mode 100644
index 0000000000000..ad2c4e6884e57
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-fundamental-integer-types.cpp
@@ -0,0 +1,141 @@
+// RUN: %check_clang_tidy %s modernize-avoid-fundamental-integer-types %t
+
+// Mock fixed-width integer types
+// NOLINTBEGIN(modernize-avoid-fundamental-integer-types)
+typedef unsigned int uint32_t;
+typedef int int32_t;
+typedef unsigned long long uint64_t;
+typedef long long int64_t;
+
+// Mock standard library semantic types
+typedef unsigned long size_t;
+typedef long ptrdiff_t;
+// NOLINTEND(modernize-avoid-fundamental-integer-types)
+
+// Test fundamental integer types that should trigger warnings
+int global_int = 42;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: avoid using platform-dependent fundamental integer type 'int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+short global_short = 10;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: avoid using platform-dependent fundamental integer type 'short'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+long global_long = 100L;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: avoid using platform-dependent fundamental integer type 'long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+long long global_long_long = 1000LL;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: avoid using platform-dependent fundamental integer type 'long long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+unsigned int global_unsigned_int = 42U;
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using platform-dependent fundamental integer type 'unsigned int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+unsigned short global_unsigned_short = 10U;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: avoid using platform-dependent fundamental integer type 'unsigned short'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+unsigned long global_unsigned_long = 100UL;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: avoid using platform-dependent fundamental integer type 'unsigned long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+unsigned long long global_unsigned_long_long = 1000ULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: avoid using platform-dependent fundamental integer type 'unsigned long long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+// Test semantic types that should NOT trigger warnings
+char global_char = 'a';
+signed char global_signed_char = 'b';
+unsigned char global_unsigned_char = 'c';
+bool global_bool = true;
+wchar_t global_wchar = L'w';
+
+// Test fixed-width types that should NOT trigger warnings
+uint32_t global_uint32 = 42U;
+int32_t global_int32 = 42;
+uint64_t global_uint64 = 100ULL;
+int64_t global_int64 = 100LL;
+
+// Test semantic standard library types that should NOT trigger warnings
+size_t global_size = 100;
+ptrdiff_t global_ptrdiff = 50;
+
+// Test function parameters
+void function_with_int_param(int param) {
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: avoid using platform-dependent fundamental integer type 'int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+}
+
+void function_with_short_param(short param) {
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: avoid using platform-dependent fundamental integer type 'short'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+}
+
+// Test function return types
+int function_returning_int() {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: avoid using platform-dependent fundamental integer type 'int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+ return 42;
+}
+
+long function_returning_long() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: avoid using platform-dependent fundamental integer type 'long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+ return 100L;
+}
+
+// Test local variables
+void test_local_variables() {
+ int local_int = 10;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: avoid using platform-dependent fundamental integer type 'int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+ short local_short = 5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: avoid using platform-dependent fundamental integer type 'short'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+ unsigned long local_unsigned_long = 200UL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: avoid using platform-dependent fundamental integer type 'unsigned long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+ // These should not trigger warnings
+ char local_char = 'x';
+ bool local_bool = false;
+
+ // Fixed-width types should not trigger warnings
+ uint32_t local_uint32 = 42U;
+ int64_t local_int64 = 100LL;
+
+ // Standard library semantic types should not trigger warnings
+ size_t local_size = 200;
+ ptrdiff_t local_ptrdiff = 10;
+}
+
+// Test struct/class members
+struct TestStruct {
+ int member_int;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: avoid using platform-dependent fundam...
[truncated]
|
|
@llvm/pr-subscribers-clang-tools-extra Author: JJ Marr (jj-marr) ChangesThe fundamental types are widely recognized to be flawed because they're of implementation-defined sizes without a common purpose. For example, To address this, C/C++ introduced the TODO:
Patch is 22.04 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146970.diff 7 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.cpp
new file mode 100644
index 0000000000000..b5e2484e6396a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.cpp
@@ -0,0 +1,181 @@
+//===--- AvoidFundamentalIntegerTypesCheck.cpp - clang-tidy ---------------===//
+//
+// 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 "AvoidFundamentalIntegerTypesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+namespace {
+
+AST_MATCHER(clang::TypeLoc, hasValidBeginLoc) {
+ return Node.getBeginLoc().isValid();
+}
+
+AST_MATCHER_P(clang::TypeLoc, hasType,
+ clang::ast_matchers::internal::Matcher<clang::Type>,
+ InnerMatcher) {
+ const clang::Type *TypeNode = Node.getTypePtr();
+ return TypeNode != nullptr &&
+ InnerMatcher.matches(*TypeNode, Finder, Builder);
+}
+
+} // namespace
+
+AvoidFundamentalIntegerTypesCheck::AvoidFundamentalIntegerTypesCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+bool AvoidFundamentalIntegerTypesCheck::isFundamentalIntegerType(
+ const Type *T) const {
+ if (!T->isBuiltinType())
+ return false;
+
+ const auto *BT = T->getAs<BuiltinType>();
+ if (!BT)
+ return false;
+
+ switch (BT->getKind()) {
+ case BuiltinType::Int:
+ case BuiltinType::UInt:
+ case BuiltinType::Short:
+ case BuiltinType::UShort:
+ case BuiltinType::Long:
+ case BuiltinType::ULong:
+ case BuiltinType::LongLong:
+ case BuiltinType::ULongLong:
+ return true;
+ default:
+ return false;
+ }
+}
+
+bool AvoidFundamentalIntegerTypesCheck::isSemanticType(const Type *T) const {
+ if (!T->isBuiltinType())
+ return false;
+
+ const auto *BT = T->getAs<BuiltinType>();
+ if (!BT)
+ return false;
+
+ switch (BT->getKind()) {
+ case BuiltinType::Bool:
+ case BuiltinType::Char_S:
+ case BuiltinType::Char_U:
+ case BuiltinType::SChar:
+ case BuiltinType::UChar:
+ case BuiltinType::WChar_S:
+ case BuiltinType::WChar_U:
+ case BuiltinType::Char8:
+ case BuiltinType::Char16:
+ case BuiltinType::Char32:
+ return true;
+ default:
+ return false;
+ }
+}
+
+void AvoidFundamentalIntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
+ // Match variable declarations with fundamental integer types
+ Finder->addMatcher(
+ varDecl().bind("var_decl"),
+ this);
+
+ // Match function declarations with fundamental integer return types
+ Finder->addMatcher(
+ functionDecl().bind("func_decl"),
+ this);
+
+ // Match function parameters with fundamental integer types
+ Finder->addMatcher(
+ parmVarDecl().bind("param_decl"),
+ this);
+
+ // Match field declarations with fundamental integer types
+ Finder->addMatcher(
+ fieldDecl().bind("field_decl"),
+ this);
+
+ // Match typedef declarations to check their underlying types
+ Finder->addMatcher(
+ typedefDecl().bind("typedef_decl"),
+ this);
+
+ Finder->addMatcher(
+ typeAliasDecl().bind("alias_decl"),
+ this);
+}
+
+void AvoidFundamentalIntegerTypesCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ SourceLocation Loc;
+ QualType QT;
+ std::string DeclType;
+
+ if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("var_decl")) {
+ Loc = VD->getLocation();
+ QT = VD->getType();
+ DeclType = "variable";
+ } else if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("func_decl")) {
+ Loc = FD->getLocation();
+ QT = FD->getReturnType();
+ DeclType = "function return type";
+ } else if (const auto *PD = Result.Nodes.getNodeAs<ParmVarDecl>("param_decl")) {
+ Loc = PD->getLocation();
+ QT = PD->getType();
+ DeclType = "function parameter";
+ } else if (const auto *FD = Result.Nodes.getNodeAs<FieldDecl>("field_decl")) {
+ Loc = FD->getLocation();
+ QT = FD->getType();
+ DeclType = "field";
+ } else if (const auto *TD = Result.Nodes.getNodeAs<TypedefDecl>("typedef_decl")) {
+ Loc = TD->getLocation();
+ QT = TD->getUnderlyingType();
+ DeclType = "typedef";
+ } else if (const auto *AD = Result.Nodes.getNodeAs<TypeAliasDecl>("alias_decl")) {
+ Loc = AD->getLocation();
+ QT = AD->getUnderlyingType();
+ DeclType = "type alias";
+ } else {
+ return;
+ }
+
+ if (Loc.isInvalid() || QT.isNull())
+ return;
+
+ // Check if the type is already a typedef - if so, don't warn
+ // since the user is already using a typedef (which is what we want)
+ if (QT->getAs<TypedefType>()) {
+ return;
+ }
+
+ const Type *T = QT.getCanonicalType().getTypePtr();
+ if (!T)
+ return;
+
+ // Skip if not a fundamental integer type
+ if (!isFundamentalIntegerType(T))
+ return;
+
+ // Skip semantic types
+ if (isSemanticType(T))
+ return;
+
+ // Get the type name for the diagnostic
+ std::string TypeName = QT.getAsString();
+
+ diag(Loc, "avoid using platform-dependent fundamental integer type '%0'; "
+ "consider using a typedef or fixed-width type instead")
+ << TypeName;
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.h b/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.h
new file mode 100644
index 0000000000000..c1e4fb4748e5d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.h
@@ -0,0 +1,44 @@
+//===--- AvoidFundamentalIntegerTypesCheck.h - clang-tidy -------*- C++ -*-===//
+//
+// 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_MODERNIZE_AVOIDFUNDAMENTALINTEGERTYPESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDFUNDAMENTALINTEGERTYPESCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::modernize {
+
+/// Find fundamental integer types and recommend using typedefs or fixed-width types.
+///
+/// Detects fundamental integer types (int, short, long, long long, and their
+/// unsigned variants) and warns against their use due to platform-dependent
+/// behavior. Excludes semantic types like char, bool, wchar_t, char16_t,
+/// char32_t, size_t, and ptrdiff_t.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/avoid-fundamental-integer-types.html
+class AvoidFundamentalIntegerTypesCheck : public ClangTidyCheck {
+public:
+ AvoidFundamentalIntegerTypesCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus11;
+ }
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+
+private:
+ bool isFundamentalIntegerType(const Type *T) const;
+ bool isSemanticType(const Type *T) const;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDFUNDAMENTALINTEGERTYPESCHECK_H
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 619a27b2f9bb6..deb37c1ad9fb3 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyModernizeModule STATIC
AvoidBindCheck.cpp
AvoidCArraysCheck.cpp
+ AvoidFundamentalIntegerTypesCheck.cpp
ConcatNestedNamespacesCheck.cpp
DeprecatedHeadersCheck.cpp
DeprecatedIosBaseAliasesCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index fdf38bc4b6308..a42d55b26a311 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "AvoidBindCheck.h"
#include "AvoidCArraysCheck.h"
+#include "AvoidFundamentalIntegerTypesCheck.h"
#include "ConcatNestedNamespacesCheck.h"
#include "DeprecatedHeadersCheck.h"
#include "DeprecatedIosBaseAliasesCheck.h"
@@ -63,6 +64,8 @@ class ModernizeModule : public ClangTidyModule {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<AvoidBindCheck>("modernize-avoid-bind");
CheckFactories.registerCheck<AvoidCArraysCheck>("modernize-avoid-c-arrays");
+ CheckFactories.registerCheck<AvoidFundamentalIntegerTypesCheck>(
+ "modernize-avoid-fundamental-integer-types");
CheckFactories.registerCheck<ConcatNestedNamespacesCheck>(
"modernize-concat-nested-namespaces");
CheckFactories.registerCheck<DeprecatedHeadersCheck>(
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5098582d0c42b..33c2911fd9ae5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -280,6 +280,7 @@ Clang-Tidy Checks
:doc:`misc-use-internal-linkage <misc/use-internal-linkage>`, "Yes"
:doc:`modernize-avoid-bind <modernize/avoid-bind>`, "Yes"
:doc:`modernize-avoid-c-arrays <modernize/avoid-c-arrays>`,
+ :doc:`modernize-avoid-fundamental-integer-types <modernize/avoid-fundamental-integer-types>`,
:doc:`modernize-concat-nested-namespaces <modernize/concat-nested-namespaces>`, "Yes"
:doc:`modernize-deprecated-headers <modernize/deprecated-headers>`, "Yes"
:doc:`modernize-deprecated-ios-base-aliases <modernize/deprecated-ios-base-aliases>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-fundamental-integer-types.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-fundamental-integer-types.rst
new file mode 100644
index 0000000000000..df511e084701f
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-fundamental-integer-types.rst
@@ -0,0 +1,111 @@
+.. title:: clang-tidy - modernize-avoid-fundamental-integer-types
+
+modernize-avoid-fundamental-integer-types
+==========================================
+
+Finds fundamental integer types and recommends using typedefs or fixed-width types instead.
+
+This check detects fundamental integer types (``int``, ``short``, ``long``, ``long long``, and their
+``unsigned`` variants) and warns against their use due to non-standard platform-dependent behavior.
+For example, ``long`` is 64 bits on Linux but 32 bits on Windows. There is no standard rationale or
+intent for the sizes of these types.
+
+Instead of fundamental types, use fixed-width types such as ``int32_t`` or implementation-defined
+types with standard semantics, e.g. ``int_fast32_t`` for the fastest integer type greater than or
+equal to 32 bits.
+
+Examples
+--------
+
+.. code-block:: c++
+
+ // Bad: platform-dependent fundamental types
+ int global_int = 42;
+ short global_short = 10;
+ long global_long = 100L;
+ unsigned long global_unsigned_long = 100UL;
+
+ void function_with_int_param(int param) {
+ // ...
+ }
+
+ int function_returning_int() {
+ return 42;
+ }
+
+ struct MyStruct {
+ int member_int;
+ long member_long;
+ };
+
+.. code-block:: c++
+
+ // Good: use fixed-width types or typedefs
+ #include <cstdint>
+
+ int32_t global_int32 = 42;
+ int16_t global_int16 = 10;
+ int64_t global_int64 = 100L;
+ uint64_t global_uint64 = 100UL;
+
+ void function_with_int32_param(int32_t param) {
+ // ...
+ }
+
+ int32_t function_returning_int32() {
+ return 42;
+ }
+
+ struct MyStruct {
+ int32_t member_int32;
+ int64_t member_int64;
+ };
+
+The check will also warn about typedef declarations that use fundamental types as their underlying type:
+
+.. code-block:: c++
+
+ // Bad: typedef using fundamental type
+ typedef long long MyLongType;
+ using MyIntType = int;
+
+.. code-block:: c++
+
+ // Good: use descriptive names or fixed-width types
+ typedef int64_t TimestampType;
+ using CounterType = uint32_t;
+
+Rationale
+---------
+
+Fundamental integer types have platform-dependent sizes and behavior:
+
+- ``int`` is typically 32 bits on modern platforms but is only guaranteed to be 16 bits by the spec
+- ``long int`` is 32 bits on Windows but 64 bits on most Unix systems
+
+The C++ specification does not define these types beyond their minimum sizes. That means they can
+communicate intent in non-standard ways and are often needlessly incompatible. For example, ``int``
+was traditionally the word size of a given processor in 16-bit and 32-bit computing and was a
+reasonable default for performance. This is no longer true on modern 64-bit computers, but the size
+of ``int`` remains fixed at 32 bits for backwards compatibility with code that relied on a 32-bit
+implementation of ``int``.
+
+If code is explicitly relying on the size of an ``int`` being 32 bits, it is better to say so in
+the typename with ``int32_t``. Otherwise, use an appropriate implementation-defined type that
+communicates your intent.
+
+Types Not Flagged
+-----------------
+
+The following types are intentionally not flagged:
+
+- ``char``, ``signed char``, ``unsigned char`` (character types)
+- ``bool`` (boolean type)
+- Standard library typedefs like ``size_t``, ``ptrdiff_t``, or ``uint32_t``.
+- Already typedef'd types, though the check will flag the typedef itself
+
+``char`` is excluded because it is implementation-defined to always be 1 byte, regardless of the
+platform's definition of a byte.
+
+``bool`` is excluded because it can only be true or false, and is not vulnerable to overflow or
+narrowing issues that occur as a result of using implementation-defined types.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-fundamental-integer-types.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-fundamental-integer-types.cpp
new file mode 100644
index 0000000000000..ad2c4e6884e57
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-fundamental-integer-types.cpp
@@ -0,0 +1,141 @@
+// RUN: %check_clang_tidy %s modernize-avoid-fundamental-integer-types %t
+
+// Mock fixed-width integer types
+// NOLINTBEGIN(modernize-avoid-fundamental-integer-types)
+typedef unsigned int uint32_t;
+typedef int int32_t;
+typedef unsigned long long uint64_t;
+typedef long long int64_t;
+
+// Mock standard library semantic types
+typedef unsigned long size_t;
+typedef long ptrdiff_t;
+// NOLINTEND(modernize-avoid-fundamental-integer-types)
+
+// Test fundamental integer types that should trigger warnings
+int global_int = 42;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: avoid using platform-dependent fundamental integer type 'int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+short global_short = 10;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: avoid using platform-dependent fundamental integer type 'short'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+long global_long = 100L;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: avoid using platform-dependent fundamental integer type 'long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+long long global_long_long = 1000LL;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: avoid using platform-dependent fundamental integer type 'long long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+unsigned int global_unsigned_int = 42U;
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using platform-dependent fundamental integer type 'unsigned int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+unsigned short global_unsigned_short = 10U;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: avoid using platform-dependent fundamental integer type 'unsigned short'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+unsigned long global_unsigned_long = 100UL;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: avoid using platform-dependent fundamental integer type 'unsigned long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+unsigned long long global_unsigned_long_long = 1000ULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: avoid using platform-dependent fundamental integer type 'unsigned long long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+// Test semantic types that should NOT trigger warnings
+char global_char = 'a';
+signed char global_signed_char = 'b';
+unsigned char global_unsigned_char = 'c';
+bool global_bool = true;
+wchar_t global_wchar = L'w';
+
+// Test fixed-width types that should NOT trigger warnings
+uint32_t global_uint32 = 42U;
+int32_t global_int32 = 42;
+uint64_t global_uint64 = 100ULL;
+int64_t global_int64 = 100LL;
+
+// Test semantic standard library types that should NOT trigger warnings
+size_t global_size = 100;
+ptrdiff_t global_ptrdiff = 50;
+
+// Test function parameters
+void function_with_int_param(int param) {
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: avoid using platform-dependent fundamental integer type 'int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+}
+
+void function_with_short_param(short param) {
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: avoid using platform-dependent fundamental integer type 'short'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+}
+
+// Test function return types
+int function_returning_int() {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: avoid using platform-dependent fundamental integer type 'int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+ return 42;
+}
+
+long function_returning_long() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: avoid using platform-dependent fundamental integer type 'long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+ return 100L;
+}
+
+// Test local variables
+void test_local_variables() {
+ int local_int = 10;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: avoid using platform-dependent fundamental integer type 'int'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+ short local_short = 5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: avoid using platform-dependent fundamental integer type 'short'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+ unsigned long local_unsigned_long = 200UL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: avoid using platform-dependent fundamental integer type 'unsigned long'; consider using a typedef or fixed-width type instead [modernize-avoid-fundamental-integer-types]
+
+ // These should not trigger warnings
+ char local_char = 'x';
+ bool local_bool = false;
+
+ // Fixed-width types should not trigger warnings
+ uint32_t local_uint32 = 42U;
+ int64_t local_int64 = 100LL;
+
+ // Standard library semantic types should not trigger warnings
+ size_t local_size = 200;
+ ptrdiff_t local_ptrdiff = 10;
+}
+
+// Test struct/class members
+struct TestStruct {
+ int member_int;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: avoid using platform-dependent fundam...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
vbvictor
requested changes
Jul 3, 2025
clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.cpp
Outdated
Show resolved
Hide resolved
I don't think we need any fixes for this check - it's a tricky field, and we should let the user decide how to resolve these warnings.
I hope |
jj-marr
changed the title
|
@vbvictor It makes sense to provide an autofix for edit: according to this StackOverflow thread, Arduinos use "float" and "double" for 32-bit floats since there's no hardware 64-bit FPU.
Is an autofix from |
|
I will add the |
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.cpp
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.h
Outdated
Show resolved
Hide resolved
...tools-extra/docs/clang-tidy/checks/portability/avoid-platform-specific-fundamental-types.rst
Outdated
Show resolved
Hide resolved
5chmidti
reviewed
Jul 4, 2025
I haven't before now. I still think a new check is necessary.
This check will encompass usage of almost all fundamental types that are implementation-defined, including |
|
Check makes sense to me! As a first iteration it's fine without autofix but I can imagine it will be hard for a codebase to enable this check given the large amount of things to fix. Maybe users can specify a mapping of wanted types via an option? |
I was on mobile, so I couldn't compare them exhaustively, thanks for the run down. I'll do an actual review on the weekend |
I'm planning for an autofix of For
Based on sizeof the type when running Not sure about It's also not a "safe" replacement to get rid of |
You could make a universal option to configure autofixes. Make semicolon-separated list of key-value pairs, e.g. By default, we could even put fixes for all common types and let the user delete/modify what doesn't like. |
Co-authored-by: EugeneZelenko <[email protected]>
...tools-extra/docs/clang-tidy/checks/portability/avoid-platform-specific-fundamental-types.rst
Outdated
Show resolved
Hide resolved
This is a good idea but even the basic configuration I wanted to do adds a lot of scope to the PR. Autofix for |
jj-marr
commented
Jul 5, 2025
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.h
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.h
Outdated
Show resolved
Hide resolved
...tools-extra/docs/clang-tidy/checks/portability/avoid-platform-specific-fundamental-types.rst
Outdated
Show resolved
Hide resolved
jj-marr
commented
Jul 6, 2025
...tra/test/clang-tidy/checkers/portability/avoid-platform-specific-fundamental-types-chars.cpp
Outdated
Show resolved
Hide resolved
...ra/test/clang-tidy/checkers/portability/avoid-platform-specific-fundamental-types-floats.cpp
Show resolved
Hide resolved
...tools-extra/docs/clang-tidy/checks/portability/avoid-platform-specific-fundamental-types.rst
Show resolved
Hide resolved
...tools-extra/docs/clang-tidy/checks/portability/avoid-platform-specific-fundamental-types.rst
Outdated
Show resolved
Hide resolved
...tools-extra/docs/clang-tidy/checks/portability/avoid-platform-specific-fundamental-types.rst
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Jul 6, 2025
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.h
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.h
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/portability/AvoidPlatformSpecificFundamentalTypesCheck.cpp
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,69 @@ | |||
| // RUN: %check_clang_tidy %s portability-avoid-platform-specific-fundamental-types %t -- -config="{CheckOptions: [{key: portability-avoid-platform-specific-fundamental-types.WarnOnInts, value: false}, {key: portability-avoid-platform-specific-fundamental-types.WarnOnFloats, value: false}]}" -header-filter=.* -- -std=c++11 | |||
There was a problem hiding this comment.
Suggested change
| // RUN: %check_clang_tidy %s portability-avoid-platform-specific-fundamental-types %t -- -config="{CheckOptions: [{key: portability-avoid-platform-specific-fundamental-types.WarnOnInts, value: false}, {key: portability-avoid-platform-specific-fundamental-types.WarnOnFloats, value: false}]}" -header-filter=.* -- -std=c++11 | |
| // RUN: %check_clang_tidy -std=c++11-or-later %s portability-avoid-platform-specific-fundamental-types %t -- -config="{CheckOptions: [{key: portability-avoid-platform-specific-fundamental-types.WarnOnInts, value: false}, {key: portability-avoid-platform-specific-fundamental-types.WarnOnFloats, value: false}]}" -header-filter=.* |
Do we need -header-filter=.*?
| @@ -0,0 +1,104 @@ | |||
| // RUN: %check_clang_tidy %s portability-avoid-platform-specific-fundamental-types %t -- -config="{CheckOptions: [{key: portability-avoid-platform-specific-fundamental-types.WarnOnInts, value: false}, {key: portability-avoid-platform-specific-fundamental-types.WarnOnChars, value: false}]}" -header-filter=.* -- -std=c++23 | |||
There was a problem hiding this comment.
Suggested change
| // RUN: %check_clang_tidy %s portability-avoid-platform-specific-fundamental-types %t -- -config="{CheckOptions: [{key: portability-avoid-platform-specific-fundamental-types.WarnOnInts, value: false}, {key: portability-avoid-platform-specific-fundamental-types.WarnOnChars, value: false}]}" -header-filter=.* -- -std=c++23 | |
| // RUN: %check_clang_tidy -std=c++23-or-later %s portability-avoid-platform-specific-fundamental-types %t -- -config="{CheckOptions: [{key: portability-avoid-platform-specific-fundamental-types.WarnOnInts, value: false}, {key: portability-avoid-platform-specific-fundamental-types.WarnOnChars, value: false}]}" -header-filter=.* |
| @@ -0,0 +1,147 @@ | |||
| // RUN: %check_clang_tidy %s portability-avoid-platform-specific-fundamental-types %t -- -config="{CheckOptions: [{key: portability-avoid-platform-specific-fundamental-types.WarnOnChars, value: false}, {key: portability-avoid-platform-specific-fundamental-types.WarnOnFloats, value: false}]}" -header-filter=.* -- -std=c++11 | |||
There was a problem hiding this comment.
Suggested change
| // RUN: %check_clang_tidy %s portability-avoid-platform-specific-fundamental-types %t -- -config="{CheckOptions: [{key: portability-avoid-platform-specific-fundamental-types.WarnOnChars, value: false}, {key: portability-avoid-platform-specific-fundamental-types.WarnOnFloats, value: false}]}" -header-filter=.* -- -std=c++11 | |
| // RUN: %check_clang_tidy -std=c++11-or-later %s portability-avoid-platform-specific-fundamental-types %t -- -config="{CheckOptions: [{key: portability-avoid-platform-specific-fundamental-types.WarnOnChars, value: false}, {key: portability-avoid-platform-specific-fundamental-types.WarnOnFloats, value: false}]}" -header-filter=.* |
Comment on lines
+46
to
+47
| std::string getFloatReplacement(const BuiltinType *BT, | ||
| ASTContext &Context) const; |
There was a problem hiding this comment.
If function doesn't need check fields move it to .cpp with static
There was a problem hiding this comment.
I'm used to having linters flag this (along with missed const declarations).
There was a problem hiding this comment.
For checkers directory, I think we could have a more strict clang-tidy config with mist-const-correctness and readability-convert-member-functions-to-static enabled. I'm planning to make an RFC for it. For now sorry for the inconvenience.
|
|
||
| // Add integer types if the option is enabled | ||
| if (WarnOnInts) { | ||
| TypeStrings.insert(TypeStrings.end(), {"short", |
There was a problem hiding this comment.
The approach with type-names seems bloated and error-prone IMO, could we reuse some logic from IntegerTypesCheck.cpp? This one:
switch (BuiltinLoc.getTypePtr()->getKind()) {
case BuiltinType::Short:
case BuiltinType::Long:
case BuiltinType::LongLong:
case BuiltinType::UShort:
case BuiltinType::ULong:
case BuiltinType::ULongLong:
return true;
default:
return false;
}There was a problem hiding this comment.
You could create 3 custom matchers for int, float, double and later add them like this:
qualType(allOf(
builtinType(),
anyOf(
WarnOnInts ? isBuiltinInt() : unless(anything()),
WarnOnFloats ? isBuiltinFloat() : unless(anything()),
WarnOnFloats ? isBuiltinChar() : unless(anything())
)
))There was a problem hiding this comment.
This makes more sense. I was thinking I couldn't check the type itself because typedefs don't create a new type, but if it works for the Google check I suppose it'll work for this one.
Comment on lines
+123
to
+125
| if (TypeStrings.empty()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Suggested change
| if (TypeStrings.empty()) { | |
| return; | |
| } | |
| if (TypeStrings.empty()) | |
| return; |
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements. There are other places too, please correct.
You could enable https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm in your clang-format file.
…FundamentalTypesCheck.cpp Co-authored-by: Baranov Victor <[email protected]>
To be honest, this is because I coded 98% of this with AI (except for the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The fundamental types are widely recognized to be flawed because of implementation-defined sizes without a common purpose.
For example,
long intis a footgun since its 32 bits on Windows and 64 bits on most 64-bit Unixes. On Windows, backwards compatibility has it frozen at 32 bits while on Unix, it is implementation-defined to be the processor's word size.To address this, C/C++ introduced the
stdint.h/cstdintheaders that provide standard types of a specific number of bits. The language also introduced implementation-defined types such assize_torint_fast32_t, which unlikeint, have a clear specification of intent in the standard and can change size based on standardized aspects of the implementation (maximum size of objects, "fastest" type).Likewise, C++23 introduced
stdfloatwhich does the same for floating-point types.There is also
char8_tfor actual Unicode characters. Unlikechar, this must be at least 8 bits and is guaranteed to be unsigned. For representing bytes,std::byteis another replacement forcharand is also guaranteed to be unsigned.This linter check warns against using fundamental types in favour of standardized ones. It provides autofixes for floats as those can be usually replaced by
stdfloatin most cases. Autofixes forintwill be provided in a future PR due to the need for additional configuration options.