https://github.com/lucasly-ba updated https://github.com/llvm/llvm-project/pull/210548
>From c021701c8a050a237dad87cb4dd57a1328a45c70 Mon Sep 17 00:00:00 2001 From: Lucas Ly Ba <[email protected]> Date: Sat, 18 Jul 2026 23:09:03 +0200 Subject: [PATCH] [clang-tidy] Add `portability-errno-comparison` check Comparing 'errno' against an integer literal is not portable because the values of the error constants are implementation-defined. Flag such comparisons and point to the 'E'-prefixed macros instead. Comparisons with 0 (the standard "no error" value) and comparisons whose literal comes from a macro such as errno == EINVAL are not flagged. Fixes #182518 --- .../clang-tidy/portability/CMakeLists.txt | 1 + .../portability/ErrnoComparisonCheck.cpp | 54 +++++++++++++++++++ .../portability/ErrnoComparisonCheck.h | 37 +++++++++++++ .../portability/PortabilityTidyModule.cpp | 3 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/portability/errno-comparison.rst | 19 +++++++ .../checkers/portability/errno-comparison.c | 29 ++++++++++ .../checkers/portability/errno-comparison.cpp | 20 +++++++ 9 files changed, 169 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/portability/ErrnoComparisonCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/portability/ErrnoComparisonCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/portability/errno-comparison.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/portability/errno-comparison.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/portability/errno-comparison.cpp diff --git a/clang-tools-extra/clang-tidy/portability/CMakeLists.txt b/clang-tools-extra/clang-tidy/portability/CMakeLists.txt index 170fedf52130e..2075d225722a9 100644 --- a/clang-tools-extra/clang-tidy/portability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/portability/CMakeLists.txt @@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangTidyPortabilityModule STATIC AvoidPragmaOnceCheck.cpp + ErrnoComparisonCheck.cpp NoAssemblerCheck.cpp PortabilityTidyModule.cpp RestrictSystemIncludesCheck.cpp diff --git a/clang-tools-extra/clang-tidy/portability/ErrnoComparisonCheck.cpp b/clang-tools-extra/clang-tidy/portability/ErrnoComparisonCheck.cpp new file mode 100644 index 0000000000000..bf927d83156ac --- /dev/null +++ b/clang-tools-extra/clang-tidy/portability/ErrnoComparisonCheck.cpp @@ -0,0 +1,54 @@ +//===----------------------------------------------------------------------===// +// +// 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 "ErrnoComparisonCheck.h" +#include "clang/AST/Expr.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::portability { + +void ErrnoComparisonCheck::registerMatchers(MatchFinder *Finder) { + // Match a comparison that has an integer literal on one side. + Finder->addMatcher(binaryOperator(isComparisonOperator(), + hasEitherOperand(ignoringParenImpCasts( + integerLiteral().bind("lit")))) + .bind("cmp"), + this); +} + +void ErrnoComparisonCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Cmp = Result.Nodes.getNodeAs<BinaryOperator>("cmp"); + const auto *Lit = Result.Nodes.getNodeAs<IntegerLiteral>("lit"); + + // errno == 0 is the portable way to test for "no error". + if (Lit->getValue() == 0) + return; + + // A macro literal (e.g. EINVAL) is already the recommended form. + if (Lit->getBeginLoc().isMacroID()) + return; + + const SourceManager &SM = *Result.SourceManager; + const auto IsErrno = [&](const Expr *E) { + const SourceLocation Loc = E->getBeginLoc(); + return Loc.isMacroID() && + Lexer::getImmediateMacroName(Loc, SM, getLangOpts()) == "errno"; + }; + + // Warn only when exactly one operand is errno. + if (IsErrno(Cmp->getLHS()) == IsErrno(Cmp->getRHS())) + return; + + diag(Cmp->getOperatorLoc(), + "comparing 'errno' against a literal is not portable"); +} + +} // namespace clang::tidy::portability diff --git a/clang-tools-extra/clang-tidy/portability/ErrnoComparisonCheck.h b/clang-tools-extra/clang-tidy/portability/ErrnoComparisonCheck.h new file mode 100644 index 0000000000000..c4daec6c78d94 --- /dev/null +++ b/clang-tools-extra/clang-tidy/portability/ErrnoComparisonCheck.h @@ -0,0 +1,37 @@ +//===----------------------------------------------------------------------===// +// +// 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_ERRNOCOMPARISONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_ERRNOCOMPARISONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::portability { + +/// Flags comparisons of 'errno' against an integer literal. The values of the +/// error constants are implementation-defined, so a literal such as +/// 'errno == 5' is not portable; the 'E'-prefixed macros should be used. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/portability/errno-comparison.html +class ErrnoComparisonCheck : public ClangTidyCheck { +public: + ErrnoComparisonCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + // Match the code as written so a comparison in a template is reported once, + // not for every instantiation. + return TK_IgnoreUnlessSpelledInSource; + } +}; + +} // namespace clang::tidy::portability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_ERRNOCOMPARISONCHECK_H diff --git a/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp index 1f2340502f685..d184d00c62d36 100644 --- a/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp @@ -9,6 +9,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "AvoidPragmaOnceCheck.h" +#include "ErrnoComparisonCheck.h" #include "NoAssemblerCheck.h" #include "RestrictSystemIncludesCheck.h" #include "SIMDIntrinsicsCheck.h" @@ -24,6 +25,8 @@ class PortabilityModule : public ClangTidyModule { void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck<AvoidPragmaOnceCheck>( "portability-avoid-pragma-once"); + CheckFactories.registerCheck<ErrnoComparisonCheck>( + "portability-errno-comparison"); CheckFactories.registerCheck<NoAssemblerCheck>("portability-no-assembler"); CheckFactories.registerCheck<RestrictSystemIncludesCheck>( "portability-restrict-system-includes"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 69c3bcf67b8db..d8ca275eab9be 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -97,6 +97,11 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`portability-errno-comparison + <clang-tidy/checks/portability/errno-comparison>` check. + + Flags comparisons of ``errno`` against an integer literal. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2a44dc78fbc89..12c8f02cfa22c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -373,6 +373,7 @@ Clang-Tidy Checks :doc:`performance-unnecessary-value-param <performance/unnecessary-value-param>`, "Yes" :doc:`performance-use-std-move <performance/use-std-move>`, "Yes" :doc:`portability-avoid-pragma-once <portability/avoid-pragma-once>`, + :doc:`portability-errno-comparison <portability/errno-comparison>`, :doc:`portability-no-assembler <portability/no-assembler>`, :doc:`portability-restrict-system-includes <portability/restrict-system-includes>`, "Yes" :doc:`portability-simd-intrinsics <portability/simd-intrinsics>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/portability/errno-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/portability/errno-comparison.rst new file mode 100644 index 0000000000000..f279b62837fe7 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/portability/errno-comparison.rst @@ -0,0 +1,19 @@ +.. title:: clang-tidy - portability-errno-comparison + +portability-errno-comparison +============================ + +Flags comparisons of ``errno`` against an integer literal. + +The values of the ``errno`` error constants are implementation-defined, so +comparing ``errno`` against a hard-coded number such as ``errno == 5`` is not +portable. Use the ``E``-prefixed macros (e.g. ``EINVAL``) instead. + +.. code-block:: c + + if (errno == 5) {} // warning + if (errno == EINVAL) {} // ok, compared against the named macro + if (errno == 0) {} // ok, 0 is the standard "no error" value + +Comparisons with ``0`` and comparisons whose literal comes from a macro (such as +the ``E``-prefixed constants themselves) are not flagged. diff --git a/clang-tools-extra/test/clang-tidy/checkers/portability/errno-comparison.c b/clang-tools-extra/test/clang-tidy/checkers/portability/errno-comparison.c new file mode 100644 index 0000000000000..63a0d330b35ac --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/portability/errno-comparison.c @@ -0,0 +1,29 @@ +// RUN: %check_clang_tidy %s portability-errno-comparison %t + +extern int *__errno_location(void); +#define errno (*__errno_location()) +#define EINVAL 22 + +void positive(void) { + if (errno == 5) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: comparing 'errno' against a literal is not portable [portability-errno-comparison] + if (errno != 5) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: comparing 'errno' against a literal is not portable [portability-errno-comparison] + if (errno < 10) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: comparing 'errno' against a literal is not portable [portability-errno-comparison] + if (5 == errno) {} + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: comparing 'errno' against a literal is not portable [portability-errno-comparison] +} + +enum Err { MyErr = 5 }; + +#define CMP_ERR(e) ((e) == 5) + +void negative(int x) { + if (errno == 0) {} // errno == 0 is the portable "no error" check + if (errno != 0) {} + if (errno == EINVAL) {} // comparing against the named macro is the fix + if (errno == MyErr) {} // an enumerator is not an integer literal + if (x == 5) {} // not errno + if (CMP_ERR(errno)) {} // the comparison itself is written in a macro +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/portability/errno-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/portability/errno-comparison.cpp new file mode 100644 index 0000000000000..465bbc37fbb09 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/portability/errno-comparison.cpp @@ -0,0 +1,20 @@ +// RUN: %check_clang_tidy %s portability-errno-comparison %t + +extern int *__errno_location(); +#define errno (*__errno_location()) + +// A literal comparison written directly still fires in C++. +void direct() { + if (errno == 5) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: comparing 'errno' against a literal is not portable [portability-errno-comparison] +} + +// Comparing against a template parameter is not a literal as written, so no +// warning is issued and the instantiations below don't report it repeatedly. +template <int N> +bool cmp() { + return errno == N; +} + +bool a = cmp<5>(); +bool b = cmp<7>(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
