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

[Chris Bowler via Phabricator via libcxx-commits]

cebowleratibm added inline comments.


================
Comment at: libcxx/include/ios:709
+        static const bool value = 
+            (WEOF >= (wint_t)WCHAR_MIN || WEOF <= (wint_t)WCHAR_MAX);
+    };
----------------
cebowleratibm wrote:
> 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.
I forgot the reason we chose not to use the OptionalFill/SentinelFill suggestion is that it may affect the layout of derived classes, compared to appending the `__is_set_` member directly in `basic_ios`

I'm now planning a tweaked version of the current revision that uses sizeof but continues to use the zero-length/size 1 array member.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124555