[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
MyDeveloperDay added inline comments. Herald added a project: clang. Comment at: clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.h:1 +//===--- MixedIntArithmeticCheck.h - clang-tidy--*- C++ -*-===// +// I guess the license has to be updated Comment at: docs/ReleaseNotes.rst:247 removal of the ``const`` keyword. +>>> master remove Comment at: docs/clang-tidy/checks/cppcoreguidelines-mixed-int-arithmetic.rst:11 + +Because of the subtile difference between ``signed`` and ``unsigned`` integer +types in C++ it is recommended to use ``signed`` types in general for arithmetic probably needs running though validate_check.py from { D55523} to make this 80 chars wide Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40854/new/ https://reviews.llvm.org/D40854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth updated this revision to Diff 182353. JonasToth added a comment. - avoid bitrot Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40854/new/ 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:
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth updated this revision to Diff 181877. JonasToth added a comment. - Merge branch 'master' into check_mixed_arithmetic, a lot has happened... Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40854/new/ 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
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
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
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth updated this revision to Diff 159064. JonasToth added a comment. - get check up to date 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:
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth updated this revision to Diff 139706. JonasToth added a comment. - update to trunk - [Doc] update to new doc style 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
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth added a comment. The guideline authors will focus on mixing integer length and integer signdness. > Thank you for the suggestion! Per our editor's discussion, we agree with "If > one then mixes integer lengths and signedness in calculations confusion on > what happens might occur" and would like to focus the discussion there. > signed and unsigned are defined in the standard and we cannot quibble with > those but we can discuss mixed arithmetic. We should consider this separately. > > Bjarne will add some text to this item. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth updated this revision to Diff 129693. JonasToth added a comment. rebase to 7.0.0 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:
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth updated this revision to Diff 128870. JonasToth added a comment. - rebase after release of 6.0 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 + //
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth added a comment. Guidelines issue is here: https://github.com/isocpp/CppCoreGuidelines/issues/1120 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
aaron.ballman added a comment. In https://reviews.llvm.org/D40854#950640, @JonasToth wrote: > > The "enforcement" listed on the C++ Core Guidelines is very unhelpful, so > > it might be worth bringing it up as an issue on their bug tracker. ES.100 > > basically says "you know what we mean, wink wink" as enforcement and > > doesn't give any guidance as to what is safe or unsafe. It gives no > > exceptions, which I think is unlikely to be useful to most developers. For > > instance: `void f(unsigned i) { if (i > 0) {} }`, this is a mixed signed > > and unsigned comparison (literals are signed by default) -- should this > > check diagnose? > > I think the guidelines mostly aim for actual calculations here. For ES.102 > there is an exception to "we want actual modulo arithmetic". They seem > mostly concerned with the mixing of the signedness of integer types that > comes from explicitly expressing non-negative values with `unsigned` types. > Because this is a common pattern (e.g. STL containers), the mixing from > literals and co is quickly overseen. > See `ES.106: Don’t try to avoid negative values by using unsigned` > > > How about `unsigned int i = 12; i += 1;`? ES.102 is equally as strange with > > "Flag unsigned literals (e.g. -2) used as container subscripts." That's a > > signed literal (2) with a negation operator -- do they mean "Flag container > > subscripts whose value is known to be negative", or something else? > > I think here ES.106 is again the rationale but your example shows a hole. > `unsigned int i = -1` is explicitly forbidden and the example shows a chain > of implicit conversion of integer types as bad code. > > My gut feeling is that the guidelines want programers to write `auto i = 12u` > or `unsigned int = 12u` but they are not clear about that. Other rules that > could relate to this are: > `ES.11: Use auto to avoid redundant repetition of type names`, `ES.23: > Prefer the {} initializer syntax` (forbidding implicit conversions in > initialization), `ES.48: Avoid casts` (maybe, but implicit conversion are not > covered there). > > I will ask them about this issue and hopefully we have some clarification. > But I didn't have much success before :D Thanks! As it stands, I'm not able to determine whether your implementation actually does or does not enforce those rules. That's why I was wondering what the extent of the coverage is according to the rule authors, because I am not certain whether we're actually implementing the intent of the rules or not. > @aaron.ballman You are a maintainer for the `cert` rules, are you? How do you > handle these common issues with integer types? Maybe we could already propose > some guidance based on `cert`. It is mentioned as well written standard in > the guidelines :) Yes, I used to maintain the CERT rules when I worked for the SEI (and I authored a considerable number of them). CERT and the C++ Core Guidelines have a somewhat fundamental difference in opinion on how we write the rules. CERT flags things that are demonstrably going to lead to incorrectness (specifically, vulnerabilities). The C++ Core Guidelines flag things that could possibly lead to correctness issues but may also be perfectly correct code. Because of this difference, CERT rules are a bit harder to implement in clang-tidy because they often will have some component of data or flow analysis to them, while C++ Core Guidelines are often blanket bans. That said, CERT's guidance on integers mostly falls back to the C rules: https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152052 I would recommend looking at the C++ Core Guideline wording and try to come up with example code that would run afoul of the rule to see if *you* think a diagnostic would be useful with that code. If you can't convince yourself that diagnosing the code is a good idea, it's worth bringing up to the authors to see if they agree. Similarly, see if you can find example code that would not run afoul of the rules but you think *should* to see if they agree. (You can use the CERT rules on integers to help give ideas.) Once you have a corpus of examples you want to see diagnosed and not diagnosed, try to devise (possibly new) wording for the enforcement section of the rule and see if the rule authors agree with the enforcement. From there, writing the check becomes a pretty simple exercise in translating the enforcement into a clang-tidy check. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth added a comment. > The "enforcement" listed on the C++ Core Guidelines is very unhelpful, so it > might be worth bringing it up as an issue on their bug tracker. ES.100 > basically says "you know what we mean, wink wink" as enforcement and doesn't > give any guidance as to what is safe or unsafe. It gives no exceptions, which > I think is unlikely to be useful to most developers. For instance: `void > f(unsigned i) { if (i > 0) {} }`, this is a mixed signed and unsigned > comparison (literals are signed by default) -- should this check diagnose? I think the guidelines mostly aim for actual calculations here. For ES.102 there is an exception to "we want actual modulo arithmetic". They seem mostly concerned with the mixing of the signedness of integer types that comes from explicitly expressing non-negative values with `unsigned` types. Because this is a common pattern (e.g. STL containers), the mixing from literals and co is quickly overseen. See `ES.106: Don’t try to avoid negative values by using unsigned` > How about `unsigned int i = 12; i += 1;`? ES.102 is equally as strange with > "Flag unsigned literals (e.g. -2) used as container subscripts." That's a > signed literal (2) with a negation operator -- do they mean "Flag container > subscripts whose value is known to be negative", or something else? I think here ES.106 is again the rationale but your example shows a hole. `unsigned int i = -1` is explicitly forbidden and the example shows a chain of implicit conversion of integer types as bad code. My gut feeling is that the guidelines want programers to write `auto i = 12u` or `unsigned int = 12u` but they are not clear about that. Other rules that could relate to this are: `ES.11: Use auto to avoid redundant repetition of type names`, `ES.23: Prefer the {} initializer syntax` (forbidding implicit conversions in initialization), `ES.48: Avoid casts` (maybe, but implicit conversion are not covered there). I will ask them about this issue and hopefully we have some clarification. But I didn't have much success before :D @aaron.ballman You are a maintainer for the `cert` rules, are you? How do you handle these common issues with integer types? Maybe we could already propose some guidance based on `cert`. It is mentioned as well written standard in the guidelines :) Comment at: clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.cpp:31 + binaryOperator(allOf(anyOf(hasOperatorName("+"), hasOperatorName("-"), + hasOperatorName("*"), hasOperatorName("/")), + hasEitherOperand(UnsignedIntegerOperand), aaron.ballman wrote: > What about `%` or comparisons operators? I forgot `%`. Comparing `signed` and `unsigned` should be covered by front-end warnings (-Wsign-compare) Comment at: test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp:167-168 +void pure_signed() { + int SInt1 = 42u; + signed char SChar1 = 42u; + SignedEnum SE1 = SEnum1; aaron.ballman wrote: > I think these are intended to be flagged under ES.102. "Flag results of > unsigned arithmetic assigned to or printed as signed." Yes they are but that was unintended :) I should add a new section for such cases and include some logic in the matchers for it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
aaron.ballman added a comment. The "enforcement" listed on the C++ Core Guidelines is very unhelpful, so it might be worth bringing it up as an issue on their bug tracker. ES.100 basically says "you know what we mean, wink wink" as enforcement and doesn't give any guidance as to what is safe or unsafe. It gives no exceptions, which I think is unlikely to be useful to most developers. For instance: `void f(unsigned i) { if (i > 0) {} }`, this is a mixed signed and unsigned comparison (literals are signed by default) -- should this check diagnose? How about `unsigned int i = 12; i += 1;`? ES.102 is equally as strange with "Flag unsigned literals (e.g. -2) used as container subscripts." That's a signed literal (2) with a negation operator -- do they mean "Flag container subscripts whose value is known to be negative", or something else? Comment at: clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.cpp:31 + binaryOperator(allOf(anyOf(hasOperatorName("+"), hasOperatorName("-"), + hasOperatorName("*"), hasOperatorName("/")), + hasEitherOperand(UnsignedIntegerOperand), What about `%` or comparisons operators? Comment at: test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp:167-168 +void pure_signed() { + int SInt1 = 42u; + signed char SChar1 = 42u; + SignedEnum SE1 = SEnum1; I think these are intended to be flagged under ES.102. "Flag results of unsigned arithmetic assigned to or printed as signed." Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:131 + + 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 I think will be good idea to shorten description to one sentence and also make first sentence in documentation same. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth updated this revision to Diff 125614. JonasToth added a comment. - [Misc] remove iostream 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:
[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic
JonasToth created this revision. Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai, klimek. This check implements rules ES.100 and ES.102 of the CppCoreGuidelines forbidding to mix `signed` and `unsigned` integer types in arithmetic expression. It currently functions for the "normal" arithmetic but when implicit casts to its happen for `short` and `char` i have no idea how to implement that properly. Here some advice would be really helpful. 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: