JonasToth updated this revision to Diff 162451.
JonasToth added a comment.
- Merge branch 'master' into check_mixed_arithmetic
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40854
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.cpp
clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-mixed-int-arithmetic.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp
Index: test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-mixed-int-arithmetic %t
+
+enum UnsignedEnum : unsigned char {
+ UEnum1,
+ UEnum2
+};
+
+enum SignedEnum : signed char {
+ SEnum1,
+ SEnum2
+};
+
+unsigned char returnUnsignedCharacter() { return 42; }
+unsigned returnUnsignedNumber() { return 42u; }
+long returnBigNumber() { return 42; }
+float unrelatedThing() { return 42.f; }
+SignedEnum returnSignedEnum() { return SEnum1; }
+UnsignedEnum returnUnsignedEnum() { return UEnum1; }
+
+void mixed_binary() {
+ unsigned int UInt1 = 42;
+ signed int SInt1 = 42;
+ UnsignedEnum UE1 = UEnum1;
+ SignedEnum SE1 = SEnum1;
+ float UnrelatedFloat = 42.f;
+
+ // Test traditional integer types.
+ auto R1 = UInt1 + SInt1;
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:21: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:13: note: unsigned operand
+
+ int R2 = UInt1 - SInt1;
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:20: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:12: note: unsigned operand
+
+ unsigned int R3 = UInt1 * SInt1;
+ // CHECK-MESSAGES: [[@LINE-1]]:21: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:29: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:21: note: unsigned operand
+
+ unsigned int R4 = UInt1 / returnBigNumber();
+ // CHECK-MESSAGES: [[@LINE-1]]:21: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:29: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:21: note: unsigned operand
+
+ char R5 = returnUnsignedCharacter() + SInt1;
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:41: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:13: note: unsigned operand
+
+ auto R6 = SInt1 - 10u;
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:13: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:21: note: unsigned operand
+
+ auto R7 = UInt1 * 10;
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:21: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:13: note: unsigned operand
+
+ auto R8 = 10u / returnBigNumber();
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:19: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:13: note: unsigned operand
+
+ auto R9 = 10 + returnUnsignedCharacter();
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:13: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:18: note: unsigned operand
+
+ // Test enum types.
+ char R10 = returnUnsignedEnum() - SInt1;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:37: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:14: note: unsigned operand
+
+ unsigned char R11 = returnSignedEnum() * UInt1;
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:23: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:44: note: unsigned operand
+
+ char R12 = UE1 / SInt1;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:20: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:14: note: unsigned operand
+
+ unsigned char R13 = SE1 + UInt1;
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:23: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:29: note: unsigned operand
+
+ auto R14 = SE1 - 10u;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:14: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:20: note: unsigned operand
+
+ auto R15 = UE1 * 10;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:20: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:14: note: unsigned operand
+
+ auto R16 = returnSignedEnum() / 10u;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:14: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:35: note: unsigned operand
+
+ auto R17 = returnUnsignedEnum() + 10;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+ // CHECK-MESSAGES: [[@LINE-2]]:37: note: signed operand
+ // CHECK-MESSAGES: [[@LINE-3]]:14: note: unsigned operand
+
+ // Check that floating pointer numbers do not interfere improperly.
+ // Implicit conversion from float to int are covered in other checks.
+ int Ok1 = UInt1 + UnrelatedFloat;
+ unsigned int Ok2 = SInt1 - UnrelatedFloat;
+ int Ok3 = UInt1 * unrelatedThing();
+ unsigned int Ok4 = SInt1 / unrelatedThing();
+ auto Ok5 = 10 + UnrelatedFloat;
+ auto Ok6 = 10u - UnrelatedFloat;
+}
+
+void mixed_assignments() {
+}
+
+void pure_unsigned() {
+ unsigned int UInt1 = 42u;
+ unsigned char UChar1 = 42u;
+ UnsignedEnum UE1 = UEnum1;
+ float UnrelatedFloat = 42.f;
+
+ auto Ok1 = UInt1 + UChar1;
+ auto Ok2 = UChar1 + UInt1;
+ auto Ok3 = UInt1 + returnUnsignedCharacter();
+ auto Ok4 = UChar1 + returnUnsignedCharacter();
+ auto Ok5 = returnUnsignedCharacter() + returnUnsignedCharacter();
+ auto Ok6 = UInt1 + UE1;
+ auto Ok7 = UInt1 + returnUnsignedEnum();
+ auto Ok8 = UE1 + UE1;
+ // FIXME: unsigned character converts to 'int' and pollutes the result.
+ // http://en.cppreference.com/w/cpp/language/implicit_conversion Integral conversions
+ // if `returnUnsignedCharacter()` would return char, the conversion would result
+ // in either `signed int` or `unsigned int` (arch dependent i guess). Both `short`
+ // and `char` perform this conversion in arithmetic operations. This would probably
+ // need some bigger magic to match in the AST, but should be possible in theory.
+ auto Ok9 = 10u * (returnUnsignedNumber() + returnUnsignedEnum());
+ auto Ok10 = 10u * (10u + 10ul);
+ auto Ok11 = 10u * (10u + returnUnsignedEnum());
+ auto Ok12 = returnUnsignedCharacter() * (10u + returnUnsignedEnum());
+
+ // Test that unrelated types do not interfere.
+ auto OkUnrelated1 = UInt1 + UnrelatedFloat;
+ auto OkUnrelated2 = UChar1 + unrelatedThing();
+ auto OkUnrelated3 = UE1 + UnrelatedFloat;
+ auto OkUnrelated4 = UE1 + unrelatedThing();
+
+ // Test that correct assignments to not cause warnings.
+ UInt1 += 1u;
+ UInt1 -= returnUnsignedCharacter();
+ UInt1 *= UE1;
+ UInt1 /= returnUnsignedEnum();
+ UInt1 += (returnUnsignedCharacter() + returnUnsignedEnum());
+}
+
+void pure_signed() {
+ int SInt1 = 42u;
+ signed char SChar1 = 42u;
+ SignedEnum SE1 = SEnum1;
+
+ float UnrelatedFloat = 42.f;
+
+ auto Ok1 = SInt1 + SChar1;
+ auto Ok2 = SChar1 + SInt1;
+ auto Ok3 = SInt1 + returnBigNumber();
+ auto Ok4 = SChar1 + returnBigNumber();
+ auto Ok5 = returnBigNumber() + returnBigNumber();
+ auto Ok6 = SInt1 + SE1;
+ auto Ok7 = SInt1 + returnSignedEnum();
+ auto Ok8 = SE1 + SE1;
+ auto Ok9 = 10 * (returnBigNumber() + returnSignedEnum());
+
+ // Test that unrelated types do not interfere.
+ auto OkUnrelated1 = SInt1 + UnrelatedFloat;
+ auto OkUnrelated2 = SChar1 + unrelatedThing();
+ auto OkUnrelated3 = SE1 + UnrelatedFloat;
+ auto OkUnrelated4 = SE1 + unrelatedThing();
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -84,6 +84,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) <cppcoreguidelines-avoid-magic-numbers>
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature>
cppcoreguidelines-interfaces-global-init
+ cppcoreguidelines-mixed-int-arithmetic
cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
Index: docs/clang-tidy/checks/cppcoreguidelines-mixed-int-arithmetic.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-mixed-int-arithmetic.rst
@@ -0,0 +1,38 @@
+.. title:: clang-tidy - cppcoreguidelines-mixed-int-arithmetic
+
+cppcoreguidelines-mixed-int-arithmetic
+======================================
+
+This check enforces `ES. 100 <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es100-dont-mix-signed-and-unsigned-arithmetic>`_
+and `ES. 102 <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es102-use-signed-types-for-arithmetic>`_
+of the `CppCoreGuidelines <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c-core-guidelines>`_
+addressing the use of unsigned types in integer arithmetic.
+
+Because of the subtile difference between ``signed`` and ``unsigned`` integer
+types in C++ it is recommended to use ``signed`` types in general for arithmetic
+and to not mix ``signed`` and ``unsigned`` integers in arithmetic expressions.
+
+The behaviour of ``signed`` integer type is undefined when an overflow occurs.
+On the contrary ``unsigned`` types will wrap around leading to potentially
+unexpected results of integer computations.
+
+.. code-block:: c++
+
+ // Taken from the CppCoreGuidelines
+ template<typename T, typename T2>
+ T subtract(T x, T2 y)
+ {
+ return x - y;
+ }
+
+ void test()
+ {
+ int s = 5;
+ unsigned int us = 5;
+ cout << subtract(s, 7) << '\n'; // -2
+ cout << subtract(us, 7u) << '\n'; // 4294967294
+ cout << subtract(s, 7u) << '\n'; // -2
+ cout << subtract(us, 7) << '\n'; // 4294967294
+ cout << subtract(s, us + 2) << '\n'; // -2
+ cout << subtract(us, s + 2) << '\n'; // 4294967294
+ }
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -70,6 +70,13 @@
Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
delimiter is a single character string literal and replaces with a character.
+- New :doc:`cppcoreguidelines-mixed-int-arithmetic
+ <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-mixed-int-arithmetic.html>` check.
+
+ Finds cases where unsigned and signed integers are used together in arithmetic expressions.
+ Unsigned integers wrap to 0 when overflowing while the behaviour of signed integers
+ is undefined in this case. The combination leads to possibly unexpected results.
+
- New :doc:`readability-magic-numbers
<clang-tidy/checks/readability-magic-numbers>` check.
Index: clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.h
@@ -0,0 +1,35 @@
+//===--- MixedIntArithmeticCheck.h - clang-tidy------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MIXED_INT_ARITHMETIC_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MIXED_INT_ARITHMETIC_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Find places that use unsigned integers in arithmetic expressions.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-mixed-int-arithmetic.html
+class MixedIntArithmeticCheck : public ClangTidyCheck {
+public:
+ MixedIntArithmeticCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MIXED_INT_ARITHMETIC_H
Index: clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.cpp
@@ -0,0 +1,65 @@
+//===--- MixedIntArithmeticCheck.cpp - clang-tidy--------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MixedIntArithmeticCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void MixedIntArithmeticCheck::registerMatchers(MatchFinder *Finder) {
+ const auto UnsignedIntegerOperand =
+ expr(ignoringImpCasts(hasType(isUnsignedInteger())))
+ .bind("unsigned-binary-operand");
+ const auto SignedIntegerOperand =
+ expr(ignoringImpCasts(hasType(isSignedInteger())))
+ .bind("signed-binary-operand");
+
+ // Match integer arithmetic mixing signed and unsigned types.
+ Finder->addMatcher(
+ binaryOperator(allOf(anyOf(hasOperatorName("+"), hasOperatorName("-"),
+ hasOperatorName("*"), hasOperatorName("/")),
+ hasEitherOperand(UnsignedIntegerOperand),
+ hasEitherOperand(SignedIntegerOperand),
+ hasRHS(hasType(isInteger())),
+ hasLHS(hasType(isInteger()))))
+ .bind("mixed-binary-arithmetic"),
+ this);
+}
+
+void MixedIntArithmeticCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *UnsignedOperand =
+ Result.Nodes.getNodeAs<Expr>("unsigned-binary-operand");
+ const auto *SignedOperand =
+ Result.Nodes.getNodeAs<Expr>("signed-binary-operand");
+ const auto *MixedArithmetic =
+ Result.Nodes.getNodeAs<BinaryOperator>("mixed-binary-arithmetic");
+
+ assert(UnsignedOperand && MixedArithmetic &&
+ "Matcher did not match operand and operation");
+
+ diag(MixedArithmetic->getLocStart(),
+ "mixed signed and unsigned arithmetic; "
+ "prefer signed integers and use unsigned types only for modulo "
+ "arithmetic")
+ << MixedArithmetic->getSourceRange();
+
+ diag(SignedOperand->getLocStart(), "signed operand", DiagnosticIDs::Note)
+ << SignedOperand->getSourceRange();
+ diag(UnsignedOperand->getLocStart(), "unsigned operand", DiagnosticIDs::Note)
+ << UnsignedOperand->getSourceRange();
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -14,6 +14,7 @@
#include "../readability/MagicNumbersCheck.h"
#include "AvoidGotoCheck.h"
#include "InterfacesGlobalInitCheck.h"
+#include "MixedIntArithmeticCheck.h"
#include "NarrowingConversionsCheck.h"
#include "NoMallocCheck.h"
#include "OwningMemoryCheck.h"
@@ -44,6 +45,8 @@
"cppcoreguidelines-interfaces-global-init");
CheckFactories.registerCheck<readability::MagicNumbersCheck>(
"cppcoreguidelines-avoid-magic-numbers");
+ CheckFactories.registerCheck<MixedIntArithmeticCheck>(
+ "cppcoreguidelines-mixed-int-arithmetic");
CheckFactories.registerCheck<NarrowingConversionsCheck>(
"cppcoreguidelines-narrowing-conversions");
CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc");
Index: clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -4,6 +4,7 @@
AvoidGotoCheck.cpp
CppCoreGuidelinesTidyModule.cpp
InterfacesGlobalInitCheck.cpp
+ MixedIntArithmeticCheck.cpp
NarrowingConversionsCheck.cpp
NoMallocCheck.cpp
OwningMemoryCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
