[clang-tidy] fix cppcoreguidelines-narrowing-conversions false positives when narrowing integer to signed integer in C++20

clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp

@@ -19,6 +19,7 @@
 #include "llvm/ADT/SmallVector.h"
 
 #include <cstdint>
+#include <optional>
 
 using namespace clang::ast_matchers;
 
@@ -48,7 +49,7 @@ NarrowingConversionsCheck::NarrowingConversionsCheck(StringRef Name,
                                                      ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       WarnOnIntegerNarrowingConversion(
-          Options.get("WarnOnIntegerNarrowingConversion", true)),
+          Options.get<bool>("WarnOnIntegerNarrowingConversion")),
       WarnOnIntegerToFloatingPointNarrowingConversion(
           Options.get("WarnOnIntegerToFloatingPointNarrowingConversion", true)),
       WarnOnFloatingPointNarrowingConversion(
@@ -61,8 +62,9 @@ NarrowingConversionsCheck::NarrowingConversionsCheck(StringRef Name,
 
 void NarrowingConversionsCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "WarnOnIntegerNarrowingConversion",
-                WarnOnIntegerNarrowingConversion);
+  if (WarnOnIntegerNarrowingConversion.has_value())
+    Options.store(Opts, "WarnOnIntegerNarrowingConversion",
+                  WarnOnIntegerNarrowingConversion.value());
   Options.store(Opts, "WarnOnIntegerToFloatingPointNarrowingConversion",
                 WarnOnIntegerToFloatingPointNarrowingConversion);
   Options.store(Opts, "WarnOnFloatingPointNarrowingConversion",
@@ -392,9 +394,15 @@ void NarrowingConversionsCheck::handleIntegralCast(const ASTContext &Context,
                                                    SourceLocation SourceLoc,
                                                    const Expr &Lhs,
                                                    const Expr &Rhs) {
-  if (WarnOnIntegerNarrowingConversion) {
+  // From [conv.integral] since C++20
+  // The result is the unique value of the destination type that is congruent
+  // to the source integer modulo 2^N, where N is the width of the destination
+  // type.
+  const bool ActualWarnOnIntegerNarrowingConversion =
+      WarnOnIntegerNarrowingConversion.value_or(!getLangOpts().CPlusPlus20);
+  if (ActualWarnOnIntegerNarrowingConversion) {
     const BuiltinType *ToType = getBuiltinType(Lhs);
-    // From [conv.integral]p7.3.8:
+    // From [conv.integral] before C++20:
     // Conversions to unsigned integer is well defined so no warning is issued.
     // "The resulting value is the smallest unsigned value equal to the source
     // value modulo 2^n where n is the number of bits used to represent the

clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h

@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_NARROWING_CONVERSIONS_H
 
 #include "../ClangTidyCheck.h"
+#include <optional>
 
 namespace clang::tidy::cppcoreguidelines {
 
@@ -95,7 +96,7 @@ class NarrowingConversionsCheck : public ClangTidyCheck {
                                           const BuiltinType &FromType,
                                           const BuiltinType &ToType) const;
 
-  const bool WarnOnIntegerNarrowingConversion;
+  const std::optional<bool> WarnOnIntegerNarrowingConversion;
   const bool WarnOnIntegerToFloatingPointNarrowingConversion;
   const bool WarnOnFloatingPointNarrowingConversion;
   const bool WarnWithinTemplateInstantiation;

clang-tools-extra/docs/ReleaseNotes.rst

@@ -207,6 +207,10 @@ Changes in existing checks
   <clang-tidy/checks/cppcoreguidelines/init-variables>` check by fixing the
   insertion location for function pointers.
 
+- Fixed :doc:`cppcoreguidelines-narrowing-conversions
+  <clang-tidy/checks/cppcoreguidelines/narrowing-conversions>` check to avoid
+  false positives when narrowing integer to signed integer in C++20.
+
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check to
   avoid false positive when member initialization depends on a structured

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/narrowing-conversions.rst

@@ -34,7 +34,9 @@ Options
 .. option:: WarnOnIntegerNarrowingConversion
 
     When `true`, the check will warn on narrowing integer conversion
-    (e.g. ``int`` to ``size_t``). `true` by default.
+    (e.g. ``int`` to ``size_t``).
+    Before C++20 `true` by default.
+    Since C++20 `false` by default.
 
 .. option:: WarnOnIntegerToFloatingPointNarrowingConversion
 
@@ -89,7 +91,9 @@ the range [-2^31, 2^31-1].
 
 You may have encountered messages like "narrowing conversion from 'unsigned int'
 to signed type 'int' is implementation-defined".
-The C/C++ standard does not mandate two's complement for signed integers, and so
-the compiler is free to define what the semantics are for converting an unsigned
-integer to signed integer. Clang's implementation uses the two's complement
-format.
+Before C++20, the C/C++ standard does not mandate two's complement for signed
+integers, and so the compiler is free to define what the semantics are for
+converting an unsigned integer to signed integer. Clang's implementation uses
+the two's complement format.
+Since C++20, the C++ standard defines the conversion between all kinds of
+integers.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-bitfields.cpp

@@ -1,5 +1,5 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
-// RUN:   -std=c++17 -- -target x86_64-unknown-linux
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t -std=c++17 \
+// RUN: -- -target x86_64-unknown-linux
 
 #define CHAR_BITS 8
 static_assert(sizeof(unsigned int) == 32 / CHAR_BITS);

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-cxx20.cpp

@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t -std=c++20 -- -Wno-everything
+
+void narrow_integer_to_signed_integer_is_ok_since_cxx20() {
+  char c;
+  short s;
+  int i;
+  long l;
+  long long ll;
+
+  unsigned char uc;
+  unsigned short us;
+  unsigned int ui;
+  unsigned long ul;
+  unsigned long long ull;
+
+  c = c;
+  c = s;
+  c = i;
+  c = l;
+  c = ll;
+
+  c = uc;
+  c = us;
+  c = ui;
+  c = ul;
+  c = ull;
+
+  i = c;
+  i = s;
+  i = i;
+  i = l;
+  i = ll;
+
+  i = uc;
+  i = us;
+  i = ui;
+  i = ul;
+  i = ull;
+
+  ll = c;
+  ll = s;
+  ll = i;
+  ll = l;
+  ll = ll;
+
+  ll = uc;
+  ll = us;
+  ll = ui;
+  ll = ul;
+  ll = ull;
+}
+
+void narrow_constant_to_signed_integer_is_ok_since_cxx20() {
+  char c1 = -128;
+  char c2 = 127;
+  char c3 = -129;
+  char c4 = 128;
+
+  short s1 = -32768;
+  short s2 = 32767;
+  short s3 = -32769;
+  short s4 = 32768;
+}

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-equivalentbitwidth-option.cpp

@@ -1,7 +1,7 @@
-// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s -std=c++17 \
 // RUN: cppcoreguidelines-narrowing-conversions %t -- 
 
-// RUN: %check_clang_tidy -check-suffix=DISABLED %s \
+// RUN: %check_clang_tidy -check-suffix=DISABLED %s -std=c++17 \
 // RUN: cppcoreguidelines-narrowing-conversions %t -- \
 // RUN: -config='{CheckOptions: { \
 // RUN:   cppcoreguidelines-narrowing-conversions.WarnOnEquivalentBitWidth: 0}}'

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp

@@ -1,7 +1,7 @@
-// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s -std=c++17 \
 // RUN: cppcoreguidelines-narrowing-conversions %t --
 
-// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s -std=c++17 \
 // RUN: cppcoreguidelines-narrowing-conversions %t -- \
 // RUN: -config='{CheckOptions: { \
 // RUN:   cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes: "global_size_t;nested_size_type;long" \

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-intemplates-option.cpp

@@ -1,7 +1,7 @@
-// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s -std=c++17 \
 // RUN: cppcoreguidelines-narrowing-conversions %t --
 
-// RUN: %check_clang_tidy -check-suffix=WARN %s \
+// RUN: %check_clang_tidy -check-suffix=WARN %s -std=c++17 \
 // RUN: cppcoreguidelines-narrowing-conversions %t -- \
 // RUN: -config='{CheckOptions: { \
 // RUN:   cppcoreguidelines-narrowing-conversions.WarnWithinTemplateInstantiation: 1 \

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-long-is-32bits.cpp

@@ -1,5 +1,5 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
-// RUN: -- -- -target x86_64-unknown-linux -m32
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t -std=c++17 \
+// RUN: -- -target x86_64-unknown-linux -m32
 
 static_assert(sizeof(int) * 8 == 32, "int is 32-bits");
 static_assert(sizeof(long) * 8 == 32, "long is 32-bits");

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-narrowinginteger-option.cpp

@@ -1,8 +1,8 @@
-// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s -std=c++17 \
 // RUN: cppcoreguidelines-narrowing-conversions %t -- \
 // RUN: -config='{CheckOptions: {cppcoreguidelines-narrowing-conversions.WarnOnIntegerNarrowingConversion: true}}'
 
-// RUN: %check_clang_tidy -check-suffix=DISABLED %s \
+// RUN: %check_clang_tidy -check-suffix=DISABLED %s -std=c++17 \
 // RUN: cppcoreguidelines-narrowing-conversions %t -- \
 // RUN: -config='{CheckOptions: {cppcoreguidelines-narrowing-conversions.WarnOnIntegerNarrowingConversion: false}}'
 

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-unsigned-char.cpp

@@ -1,5 +1,5 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
-// RUN: -- -- -target x86_64-unknown-linux -funsigned-char
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t -std=c++17 \
+// RUN: -- -target x86_64-unknown-linux -funsigned-char
 
 void narrow_integer_to_unsigned_integer_is_ok() {
   signed char sc;

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions.cpp

@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t -std=c++17 \
 // RUN: -config="{CheckOptions: { \
 // RUN:   cppcoreguidelines-narrowing-conversions.WarnOnFloatingPointNarrowingConversion: false}}" \
 // RUN: -- -target x86_64-unknown-linux -fsigned-char