Re: [boost] [outcome] Exception safety guarantees

Subject: Re: [boost] [outcome] Exception safety guarantees
From: Andrzej Krzemienski (akrzemi1_at_[hidden])
Date: 2017-05-29 07:50:11

2017-05-29 1:20 GMT+02:00 Emil Dotchevski via Boost <boost_at_[hidden]>:

> 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>.
>

Are you suggesting that one should use shared_ptrs instead of movable
types? Are you saying that the design of a movable std::fstream is wrong?

>
>
> > 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!!",

Not only overhed. also the fact that you are implying shared ownership
semantics (possibly across threads), even though you have none.

> 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.
>

Maybe if you had a weak_ptr for a unique_ptr it would convince me.

>
> 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();
> }
>

You also have one `if` here. I can also have a one-if solution with a
unique_ptr:

```
if( unique_ptr<foo> sp = get() )
{
  sp->do_a();
  sp->do_b();
}
```

It looks like the same (in)convenience to me.

> 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.
>

What about std::fstream. Does it butcher RAII?

>
> Again, granted, this is a moot point if you've already disabled exception
> handling,

No, no. I was raising doubts about the cases with exceptions as well.

which also effectively butchers RAII. Interestingly, this also
> leads to having to write ifs all over the place, this time to manually
> enforce postconditions.
>

I think this can also be dealt with one-if solution as you described.

```
if( outcome<foo> sp = foo::make() ) // factory instead constructor
{
  sp->do_a();
  sp->do_b();
}
```

Regards,
&rzej;