================
@@ -286,7 +286,33 @@ 
clang::analyze_format_string::ParseLengthModifier(FormatSpecifier &FS,
       lmKind = LengthModifier::AsInt3264;
       break;
     case 'w':
-      lmKind = LengthModifier::AsWide; ++I; break;
+      ++I;
+      if (I == E) return false;
+      if (*I == 'f') {
+        lmKind = LengthModifier::AsWideFast;
+        ++I;
+      } else {
+        lmKind = LengthModifier::AsWide;
+      }
+
+      if (I == E) return false;
+      int s = 0;
+      while (unsigned(*I - '0') <= 9) {
+        s = 10 * s + unsigned(*I - '0');
+        ++I;
+      }
+
+      // s == 0 is MSVCRT case, like l but only for c, C, s, S, or Z on windows
+      // s != 0 for b, d, i, o, u, x, or X when a size followed(like 8, 16, 32 
or 64)
+      if (s != 0) {
+        std::set<int> supported_list {8, 16, 32, 64};
----------------
enh-google wrote:

> So I think we should probably err on the side of specifying all the 
> bit-widths we specify in stdint.h.

as a libc maintainer (who happens to have done a survey of the other libcs on 
this specific bit of C23 functionality, when zijunzhao was implementing it for 
bionic :-) ), i'd argue the opposite: none of bionic, glibc, musl, FreeBSD, and 
Apple's fork of FreeBSD libc supports these weird sizes[1]. nor does any 
hardware i'm aware of. i'd actually argue that the llvm stdint.h change that 
added these types should be reverted[2]. (presumably someone who knows the llvm 
code better can check whether it's possible for clang to ever actually define 
`__INT48_TYPE__` and its non-power-of-two friends? if there really _is_ such an 
architecture, we could at least get a useful code comment in stdint.h out of 
it!)

as for the diagnostics, i'd argue (a) it doesn't make sense having this be 
libc-specific (like, for example, the existing "do math functions set errno?" 
configuration) since every libc in use would have the same "no, we don't 
support 48-bit ints" setting and (b) saying "well, 56-bit ints _might_ be a 
thing in theory, so we'll punt and leave it to be runtime error" isn't very 
helpful in a world where it will always be a runtime error.

___
1. to be fair, a couple of them still don't implement %w at all. at the risk of 
making more work for zijunzhao, if you were going to teach clang about 
different libc versions, _that_ would at least be useful (for those targets 
that include a version in them): "which version of Android/iOS first had %b?" 
etc. if i'm using %b but targeting a version that didn't have it, that's a 
useful compile-time warning, at least as long as anyone's targeting old-enough 
versions. (and, full disclosure: for Android that's the same as %w: they're 
both new in this year's release --- 
https://android.googlesource.com/platform/bionic/+/HEAD/docs/status.md)
2. personally, i don't feel like the commit message on the change that 
introduced this stuff to stdint.h motivated it at all. i suspect if llvm hadn't 
still been an academic project back then, that change would never have been 
accepted in the first place!

https://github.com/llvm/llvm-project/pull/71771
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to