SERVER-69729 Add tidy check for signed char misuse

4 files changed, 25 insertions, 6 deletions

diff --git a/.clang-tidy b/.clang-tidy
index 5c5147b7563..5915b5812e6 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -35,6 +35,7 @@ Checks:          '-*,
                   performance-faster-string-find,
                   performance-implicit-conversion-in-loop,
                   performance-inefficient-algorithm,
+                  bugprone-signed-char-misuse,
                   bugprone-suspicious-string-compare,
                   performance-for-range-copy,
                   -bugprone-argument-comment,
diff --git a/src/mongo/bson/bson_validate_test.cpp b/src/mongo/bson/bson_validate_test.cpp
index d377d380009..5a2314b8882 100644
--- a/src/mongo/bson/bson_validate_test.cpp
+++ b/src/mongo/bson/bson_validate_test.cpp
@@ -580,11 +580,21 @@ TEST(BSONValidateFast, BoolValuesAreValidated) {
     ASSERT_OK(validateBSON(obj));
     const BSONElement x = obj["x"];
     // Legal, because we know that the BufBuilder gave
-    // us back some heap memory, which isn't oringinally const.
+    // us back some heap memory, which isn't originally const.
     auto writable = const_cast<char*>(x.value());
-    for (int val = std::numeric_limits<char>::min();
-         val != (int(std::numeric_limits<char>::max()) + 1);
-         ++val) {
+    // The next few lines may look non-optimal and strange, however the assignment of a
+    // signed char to int is not safe, generally speaking. Recall that the numeric_limits
+    // library returns the limit as the type requested and that the signedness of char is
+    // platform-dependent. We know in this case that all values of a byte need to be
+    // tested, so we can hardcode the limits ourselves to sidestep casting issues and
+    // warnings from clang-tidy.
+    //
+    // Loop iteration will be the following for platforms with:
+    //    signed char: -128->127
+    //  unsigned char: 128->255, 0->127
+    int signed_char_min = -128;
+    int signed_char_max = 127;
+    for (int val = signed_char_min; val != signed_char_max + 1; ++val) {
         *writable = static_cast<char>(val);
         if ((val == 0) || (val == 1)) {
             ASSERT_OK(validateBSON(obj));
diff --git a/src/mongo/util/represent_as.h b/src/mongo/util/represent_as.h
index b491879f465..24468809ba8 100644
--- a/src/mongo/util/represent_as.h
+++ b/src/mongo/util/represent_as.h
@@ -208,6 +208,9 @@ boost::optional<Output> representAs(Input number) try {
         } else {
             return {};
         }
+    } else if constexpr ((std::is_same_v<int, Output> && std::is_same_v<signed char, Input>) ||
+                         (std::is_same_v<int, Output> && std::is_same_v<char, Input>)) {
+        return static_cast<Output>(static_cast<unsigned char>(number));
     } else {
         // If number is NaN and Output can also represent NaN, return NaN
         // Note: We need to specifically handle NaN here because of the way
diff --git a/src/mongo/util/represent_as_test.cpp b/src/mongo/util/represent_as_test.cpp
index d139dfa435b..bfd4a7bccc1 100644
--- a/src/mongo/util/represent_as_test.cpp
+++ b/src/mongo/util/represent_as_test.cpp
@@ -47,7 +47,9 @@ namespace {
 using namespace fmt::literals;
 
 // Char values
-const signed char kCharMax = std::numeric_limits<signed char>::max();
+const char kCharMax = std::numeric_limits<char>::max();
+const signed char kSCharMax = std::numeric_limits<signed char>::max();
+const int kSCharMaxAsInt = kSCharMax;
 const int kCharMaxAsInt = kCharMax;
 
 // Unsigned char values
@@ -430,9 +432,12 @@ TEST(RepresentAs, DoubleToDecimal128) {
 
 TEST(RepresentAs, PlatformDependent) {
     // signed char
-    ASSERT(*(representAs<int>(kCharMax)) == kCharMaxAsInt);
+    ASSERT(*(representAs<int>(kSCharMax)) == kSCharMaxAsInt);
     ASSERT(!(representAs<signed char>(kIntMax)));
 
+    // unspecified/bare char
+    ASSERT(*(representAs<int>(kCharMax)) == kCharMaxAsInt);
+
     // unsigned char
     ASSERT(*(representAs<int>(kUCharMax)) == kUCharMaxAsInt);
     ASSERT(!(representAs<unsigned char>(kIntMin)));