Re: [boost] [outcome] Exception safety guarantees

Subject: Re: [boost] [outcome] Exception safety guarantees
From: Emil Dotchevski (emildotchevski_at_[hidden])
Date: 2017-05-28 23:20:19

On Sun, May 28, 2017 at 3:15 PM, Andrzej Krzemienski via Boost <
boost_at_[hidden]> wrote:

> 2017-05-28 3:02 GMT+02:00 Emil Dotchevski via Boost <boost_at_[hidden]
> >:
>
> > On Sat, May 27, 2017 at 4:09 PM, Andrzej Krzemienski via Boost <
> > boost_at_[hidden]> wrote:
> >
> > > 2017-05-28 0:55 GMT+02:00 Emil Dotchevski via Boost <
> > boost_at_[hidden]
> > > >:
> > >
> > > > On Sat, May 27, 2017 at 3:15 PM, Andrzej Krzemienski via Boost <>
> Note
> > > that there is no provision to report a
> > > > failure (to establish the invariants) except by throwing. This is not
> > an
> > > > omission but a deliberate design choice.
> > > >
> > >
> > > Interesting. I do not know what you mean here. Maybe, "there is no
> other
> > > way to report such failure except to throw an exception"? If so, I
> agree,
> > > but how does this relate to the discussed topic?
> > >
> >
> > What I am demonstrating is that the C++ semantics for initialization and
> > destruction of objects have a built-in assumption about what constitutes
> a
> > valid object. Can you define this as "the only safe thing to do with x is
> > call is_valid()" on it? Sure, but then you're operating as a C
> programmer.
> > Consider what a C programmer has to do:
> >
> > struct foo { /*state*/ };
> >
> > void init_foo( foo * x )
> > {
> >    /*initialization*/
> >   assert(is_valid(x));
> > }
> >
> > void destroy_foo( foo * x )
> > {
> >   assert(is_valid(x));
> >   //destroy *x
> > }
> >
> > void use_foo( foo * x )
> > {
> >   assert(is_valid(x));
> >   /*use foo*/
> > }
> >
> > And now compare this to a C++ program:
> >
> > struct foo
> > {
> >   /*state*/
> >   foo()
> >   {
> >     /*initialization*/
> >   }
> >
> >   ~foo()
> >   {
> >     /*destruction*/
> >   }
> >
> >   void use()
> >   {
> >     /*use foo*/
> >   }
> > };
> >
> > Note that in well designed C++ programs not only the asserts are not
> > necessary, they're downright silly. Is the object valid? Duh, of course
> it
> > is, or else the constructor would not have returned. But if you
> introduce a
> > "not quite valid" state, not only you need the asserts, you're making it
> > much more difficult for the user to reason about the state of the objects
> > in his program, just like a C programmer must.
> >
>
> Emil, thanks for being patient with me. I understand what you are saying
> here. It is convincing. But ultimately I have found it to be incorrect
> after C++11 introduced move semantics. Let me show you a typical
> implementation of a RAII-like type for representing file-handles. First, in
> C++03, without moves
>
> ```
> class File
> {
>   int _handle;
>
> public:
>   explicit File(string_view name) : _handle(system::open_file(name))
>     {
>       if (_handle == 0) throw FileProblem{};
>     }
>
>   char read()
>     {
>       // no precondition: _handle always valid
>       return system::read_char(_handle);
>     }
>
>   ~File()
>     {
>       // no precondition: _handle always valid
>       system::close(_handle);
>     }
> };
> ```
>
> It is as you say: if we have an object, we know we have a file open, ready
> to be used. But now, lets's add C++11's move semantics:
>
>
> ```
> class File
> {
>   int _handle;
>
> public:
>   explicit File(string_view name) : _handle(system::open_file(name))
>     {
>       if (_handle == 0) throw FileProblem{};
>     }
>
>   File(File && rhs) : _handle(rhs._handle)
>     {
>       rhs._handle = 0; // now rhs obtains an invalid state (or, not-a-file
> state)
>     }
>
>   char read()
>     {
>       // precondition: _handle != 0
>       return system::read_char(_handle);
>     }
>
>   ~File()
>     {
>       if (_handle) // defensive if
>         system::close(_handle);
>     }
> };
> ```
>
> Now, because I have a moved-from state, it weakens all my invariants. As
> you say, every function now has a precondition: either I put defensive if's
> everywhere, or expect the users to be putting them.
>

Indeed, and this is not a good thing. Consider that the int handle doesn't
have move semantics, even though (in theory) it is possible to define some
type with invalid "moved from" state, even in C.

When dealing with things like file handles or any other resourse it's best
to use shared_ptr. This entire File class you wrote can be reduced to a
function that returns shared_ptr<int const> that closes the file in a
custom deleter. Things are even more straight-forward if you use FILE *,
then it's just shared_ptr<FILE>.

> And this is a normal moveable RAII class (or maybe a movable type is no
> longer "RAII" because of this). And we have lived with it for years now. I
> often have functions returning std::unique_ptr's, and I am not
> defensive-checking everywher if the function did not return a null. I just
> trust that if someone is returning a unique_ptr it is because they wanted
> to return a heap allocated object: not null.
>

IMO unique_ptr is way overused and shared_ptr way underused. I know,
"Overhead!!", but like in the case of exception handling the overhead is
not a problem in general and it comes with the benefit of weak_ptr.

In practice many of the concerns you expressed can be dealt with if it's
possible to hold on to the object just a bit longer until you're done with
it. Using shared/weak_ptr replaces all of the defensive ifs you otherwise
need to sprinkle around with a single if at lock time:

if( shared_ptr<foo> sp=wp.lock() )
{
  sp->do_a();
  sp->do_b();
}

Again, I understand your reasoning, but I think it equally well applies to
> moved-from state. Are you also describing shortcommings of moves?
>

Not necessarily. Note that a "moved from" std::vector is still a good
vector. I understand that sometimes it does make sense to define a less
than completely valid state and the language does support that, but these
should be treated as unfortunate necessities rather than good C++
programming practices, especially because they effectively butcher RAII.

Again, granted, this is a moot point if you've already disabled exception
handling, which also effectively butchers RAII. Interestingly, this also
leads to having to write ifs all over the place, this time to manually
enforce postconditions.