[libcxx-commits] [PATCH] D120751: [libc++] Fix initialization of __fill_

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/ios:681
     mutable int_type __fill_;
+    bool __fill_set;
 };
----------------
Quuxplusone wrote:
> Changing the size of `basic_ios` is a non-starter, because ABI.
> However, we don't need a separate `bool` for this. Notice that `eof()` by definition is not equal to any possible value of `char_type`, so if `__fill_ == traits_type::eof()` then we can be 100% sure that it hasn't been set by the user. Certainly the scenario you describe in the PR summary — "the user sets a value for the fill character that happens to `equal traits_type::eof()`" — cannot possibly happen.
This is ABI-breaking. I don't think we can fix that part of the bug. Do you know how other implementations do it? Do they use a boolean flag similarly to this? That seems like a rather strange implementation requirement to me -- is it possible that all implementations have the same bug? In that case, perhaps we only need a LWG issue to actually standardize what all implementations do, but I'm just speculating.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120751