llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: JJ Marr (jj-marr) <details> <summary>Changes</summary> The fundamental types are widely recognized to be flawed because they're of implementation-defined sizes without a common purpose. For example, `long int` is 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`/`cstdint` headers that provide standard types of a specific number of bits. The language also introduced implementation-defined types such as `size_t` or `int_fast32_t`, which unlike `int`, 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). TODO: - [ ] Decide whether to provide an autofix. I have chosen not to, because it will depend on the context one is using a type in. - [ ] Decide whether `char` should be linted. I have chosen not to lint `char` by default because it can be used to convey "1 byte" regardless of the size of the byte. However, I might add this as an option defaulted to "Yes", because on virtually every platform people actually use C++20/C23 on, `char` can be safely replaced with `char8_t` as 1 byte is 8 bits. -- If [P3477](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3477r5.html) is adopted, `CHAR_BIT=8` on all conforming C++ implementations. - [ ] Decide whether `float` and `double` should be linted in favour of [stdfloat types](https://en.cppreference.com/w/cpp/types/floating-point.html) or whether this should be a separate check. --- 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: - (added) clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.cpp (+181) - (added) clang-tools-extra/clang-tidy/modernize/AvoidFundamentalIntegerTypesCheck.h (+44) - (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1) - (modified) clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp (+3) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-fundamental-integer-types.rst (+111) - (added) clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-fundamental-integer-types.cpp (+141) ``````````diff 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] `````````` </details> https://github.com/llvm/llvm-project/pull/146970 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
