Re: [boost] Re: Boost.Threads draft library submission

From: Greg Colvin (gcolvin_at_[hidden])
Date: 2001-03-19 11:08:26

From: Peter Dimov <pdimov_at_[hidden]>
> From: "Jesse Jones" <jesjones_at_[hidden]>
> 
> > >So perhaps we need a BOOST_ASSERT(), which has at least three behaviors
> > >(determined by what preprocessor symbols are defined) when the predicate
> > >fails:
> > >
> > >   * classic assert() with -NDEBUG not defined.
> > >
> > >   * classic assert() with -NDEBUG defined.
> > >
> > >   * throw logic_error (with file/line info?).
> > >
> > >   * [possibly?] call some user or system function (with file/line
> info?).
> >
> > I think this is a good idea in any case. The C assert is plainly
> > inaequate as is evident by how often people write replacements. Something
> > this critical really belongs in a library like Boost.
> 
> I've found that the C assert is both adequate and inadequate, in the sense
> that in "usual" applications it's fine, but in "special purpose"
> applications (full screen games) it's not (I have to revert to the usual
> display mode first, in order to see the assertion failed dialog box), and
> the proposed BOOST_ASSERT won't solve the problem either.
> 
> > I still think it's better to try and partition errors into programmer and
> > system errors and handle one with assert and one with throw. Throwing is
> > part of the documentation. Programmer errors result in undefined
> > behavior, but a quality implementation will use asserts for as many of
> > these as possible and let clients modify the behavior of the assert.
> 
> This is my opinion as well. Using any kind of assert, be it BOOST_ASSERT,
> the standard C assert or another custom variant should be documented as
> "undefined behavior" in the docs, period.
> 
> To give an example:
> 
> void f1(HANDLE h)
> {
>   ::CloseHandle(h);
> }
> 
> // documentation: f1(h) closes the handle 'h', errors are silently ignored
> 
> void f2(HANDLE h)
> {
>   int err = ::CloseHandle(h);
>   assert(err == 0);
> }
> 
> // documentation: f2(h) closes the handle 'h'; inability to close the handle
> // results in undefined behavior
> 
> void f3(HANDLE h)
> {
>   if(!::CloseHandle(h)) throw std::logic_error("f3");
> }
> 
> // documentation: f3(h) closes the handle 'h'; throws std::logic error on
> error
> 
> bool f4(HANDLE h)
> {
>   return ::CloseHandle(h) == 0;
> }
> 
> // documentation: f4(h) closes the handle 'h'; returns true on success,
> false on failure
> 
> When you move the error handling into the interface specification, it
> becomes obvious that f2 isn't very good design. The client has no way to
> defend against the undefined behavior.

Except that, as I recall it, William's thread code is designed
such that CloseHandle cannot fail.  If it does fail it is a bug,
but not in the clients code.  And furthermore, the whole point
of the thread code is for the client not to have to know or care
about handles at all.  It is an artifact of the implementation
on a particular system that there is a handle at all, that it
needs closing, that it gets closed when it does, and that the
call to close can fail.

So I have no trouble with an assert in this case, as it seems
wrong to throw an exception that means "Something that was
supposed to be impossible has happened, so all bets are off.
Proceed at your own risk."

> Don't underestimate the rule "assert() only when the docs say 'undefined
> behavior'". It only _looks_ like it's a no-brainer, but has deep
> implications.

Yes it does, but I'm not sure I like them.  I generally follow
the rule "assert the invariant".