[libcxx-commits] [PATCH] D124555: [libcxx] basic_ios<wchar_t> cannot store fill character WCHAR_MAX

[Chris Bowler via Phabricator via libcxx-commits]

cebowleratibm planned changes to this revision.
cebowleratibm marked an inline comment as done.
cebowleratibm added a comment.

Writing revision to use SentinelFill/OptionalFill classes.



================
Comment at: libcxx/include/__config:319
 
+#if defined(_AIX)
+#  define _LIBCPP_BASIC_IOS_HAS_FILL_SET 1
----------------
hubert.reinterpretcast wrote:
> It is my understanding that z/OS is also in the same situation.
zOS hasn't provided a 32-bit libc++ solution yet.  They're 64-bit injected a bool a __fill_set_ member.  The compile time selection of the fill_set implementation will do the right thing for zOS 32/64 bit without need for explicit overrides.


================
Comment at: libcxx/include/ios:709
+        static const bool value = 
+            (WEOF >= (wint_t)WCHAR_MIN || WEOF <= (wint_t)WCHAR_MAX);
+    };
----------------
hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > If `wchar_t` is 16-bit unsigned, `wint_t` is 32-bit signed and `WEOF` is `-1`, then `WEOF` is less than `(wint_t)WCHAR_MAX` and `value` becomes `true` even though the new member is unneeded.
> This should detect when `wint_t` is not wider than `wchar_t`:
> ```
> WCHAR_MAX >= WINT_MAX || 1 + (wint_t)WCHAR_MAX == (wint_t)WCHAR_MIN
> ```
> but using `sizeof` might be reasonable.
I agree.  Turns out signed wchar also has a problem with -1 and @ldionne 's suggestion was the right way to go.  I'm going to use sizeof and post a revision using OptionalFill/SentinelFill suggestion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124555/new/

https://reviews.llvm.org/D124555