[libc] fix -Wtype-limits in wctob

[nickdesaulniers]

GCC warns:

libc/src/__support/wctype_utils.h:33:20: error: comparison of unsigned
expression in ‘< 0’ is always false [-Werror=type-limits]
   33 |   if (c > 127 || c < 0)
      |                  ~~^~~

Looking into the signedness of wint_t, it looks like depending on the platform,
WINT_TYPE is defined to int or unsigned int depending on the platform.

Link: https://lab.llvm.org/buildbot/#/builders/250/builds/14891/steps/6/logs/stdio

[llvmbot]

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

GCC warns:
libc/src/__support/wctype_utils.h:33:20: error: comparison of unsigned
expression in ‘< 0’ is always false [-Werror=type-limits]
33 | if (c > 127 || c < 0)
| ^

Looking into the signedness of wint_t, it looks like depending on the platform,
WINT_TYPE is defined to int or unsigned int depending on the platform.

Link: https://lab.llvm.org/buildbot/#/builders/250/builds/14891/steps/6/logs/stdio

---

Full diff: https://github.com/llvm/llvm-project/pull/74511.diff

1 Files Affected:

• (modified) libc/src/__support/wctype_utils.h (+6-1)

diff --git a/libc/src/__support/wctype_utils.h b/libc/src/__support/wctype_utils.h
index 6d825499a1b0f..367f5a86647fd 100644
--- a/libc/src/__support/wctype_utils.h
+++ b/libc/src/__support/wctype_utils.h
@@ -29,9 +29,14 @@ LIBC_INLINE cpp::optional<int> wctob(wint_t c) {
   // This needs to be translated to EOF at the callsite. This is to avoid
   // including stdio.h in this file.
   // The standard states that wint_t may either be an alias of wchar_t or
-  // an alias of an integer type, so we need to keep the c < 0 check.
+  // an alias of an integer type where different platforms define this type with
+  // different signedness, so we need to keep the c < 0 check, hence the
+  // pragmas.
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wtype-limits"
   if (c > 127 || c < 0)
     return cpp::nullopt;
+#pragma GCC diagnostic pop
   return static_cast<int>(c);
 }
 

[nickdesaulniers]

my SFINAE skills are rusty, but I wonder if std::is_signed can be used here?

[jhuber6]

my SFINAE skills are rusty, but I wonder if std::is_signed can be used here?

I was just about to comment the same thing. We should be able to do something like this

if constexpr (cpp::is_signed<wint_t>) {
} else {
}

[michaelrj-google]

LGTM with nit

[SchrodingerZhu]

@michaelrj-google sure! I only wanted to use that to remove the signedness warning :D

[nickdesaulniers]

my SFINAE skills are rusty, but I wonder if std::is_signed can be used here?

I was just about to comment the same thing. We should be able to do something like this

if constexpr (cpp::is_signed<wint_t>) {
} else {
}

cool! Done in f3a22d4

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[michaelrj-google]

This version is fine, though we could also static cast to an unsigned type and just check if the resulting value is less than 127. Either is fine, feel free to land.