Re: [lmi] [lmi-commits] master 86aacf1 5/5: Clear error flags when resetting stringstream contents

[Greg Chicares]

[icedove froze, so...rewriting my response; sorry if you get two
different replies, but if you do, they should say materially the
same thing]

On 2017-03-03 14:24, Vadim Zeitlin wrote:
> On Tue, 28 Feb 2017 19:40:47 -0500 (EST) Greg Chicares <address@hidden> wrote:

[commit details trimmed somewhat]

> GC> commit 86aacf1068183d1553f5fb9a2b2baabcd5cc1ae4
> GC>     Clear error flags when resetting stringstream contents
> GC> @@ -120,6 +120,7 @@ void concrete_progress_meter::do_dawdle(int seconds)
> GC>      for(int i = 10 * seconds; 0 < i && !progress_dialog_.WasCancelled(); 
> --i)
> GC>          {
> GC>          wxMilliSleep(100);
> GC> +        oss.clear();
> GC>          oss.str("");
> GC>          oss << "Waiting " << 0.1 * i << " seconds";
> 
>  Out of curiosity, how is it possible for this stream to get into the error
> state? AFAICS we don't do anything with it that could set either "fail" or
> "bad", do we?

Same answer as the next question--see below.

> GC> diff --git a/rtti_lmi_test.cpp b/rtti_lmi_test.cpp
> GC> index 65eb3c6..4d1d75e 100644
> GC> @@ -66,6 +66,7 @@ void RttiLmiTest::TestTypeInfo()
> GC>      ti1 = typeid(X);
> GC> +    oss.clear();
> GC>      oss.str("");
> GC>      oss << ti1;
> GC>      BOOST_TEST_EQUAL(oss.str(), lmi::detail::Demangle(typeid(X).name()));
> 
>  And here it looks like we only use "oss" to output a string to it, so
> normally it shouldn't have any state bits set...
> 
>  What am I missing?

Error-proofing. Somebody might modify the code in a way that makes it
possible for failbit or badbit to be set. If we always clear() a
std::stringstream when we .str("") it, then that combination of
operations makes it more reliably usable. And it costs almost nothing.

You might say, OTOH, that this "somebody" who makes a future modification
that can set an error flag should also test that error flag--but if they
negligently omit that test, then clear() obliterates the flag.

OTTH...maybe it would be even better to assert that the flags are clear.
But there's a negative side to that as well. Any assertion failure is
scary to an end user. And, if such an assertion fires, it might turn out
that clearing the stream state actually is the right thing to do, so, by
writing an assertion, we've scared them needlessly.