Re: [boost] [outcome] How to drop the formal empty state

Subject: Re: [boost] [outcome] How to drop the formal empty state
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2017-05-26 17:16:17

>> But if I am writing my implementation that way, it is because there is
>> no purpose here to my function returning empty. Else I would implement
>> it differently.
>>
>> I appreciate that the above is me speaking vaguely from experience and
>> without concrete evidence.
> 
> I usually place high value on argument from experience, even if
> presented without evidence.
> 
> But in this case, you've cited
> 
> https://github.com/ned14/boost.afio/blob/master/include/boost/afio/v2.0/detail/impl/posix/file_handle.ipp#L188
> 
> 
> which is this:
> 
> result<file_handle> file_handle::file(file_handle::path_type _path,
> file_handle::mode _mode, file_handle::creation _creation,
> file_handle::caching _caching, file_handle::flag flags) noexcept
> {
>  result<file_handle> ret(file_handle(native_handle_type(), 0, 0,
> std::move(_path), _caching, flags));
>  native_handle_type &nativeh = ret.get()._v;
>  BOOST_OUTCOME_TRY(attribs,
> attribs_from_handle_mode_caching_and_flags(nativeh, _mode, _creation,
> _caching, flags));
>  nativeh.behaviour |= native_handle_type::disposition::file;
>  const char *path_ = ret.value()._path.c_str();
>  nativeh.fd = ::open(path_, attribs, 0x1b0 /*660*/);
>  if(-1 == nativeh.fd)
>    return make_errored_result<file_handle>(errno,
> last190(ret.value()._path.native()));
>  BOOST_AFIO_LOG_FUNCTION_CALL(nativeh.fd);
>  if(!(flags & flag::disable_safety_unlinks))
>  {
>    BOOST_OUTCOME_TRYV(ret.value()._fetch_inode());
>  }
>  if(_creation == creation::truncate &&
> ret.value().are_safety_fsyncs_issued())
>    fsync(nativeh.fd);
>  return ret;
> }
> 
> Now, the first line constructs
> 
>  result<file_handle> ret(file_handle(native_handle_type(), 0, 0,
> std::move(_path), _caching, flags));
> 
> and it's immediately apparent that returning `ret` in this initial state
> makes no sense. That is, you either need to further initialize it into a
> valid file_handle, or return an "errored" result.

Correct. AFIO v2 uses a two-phase initialisation design pattern
throughout. The first phase is always a constexpr noexcept constructor
which ONLY ever initialises trivial types, and NEVER can error. The
newly constructed object is valid, but not useful. The second phase
occurs as the static init function runs its course, and is able to bail
out due to error at any time whereupon file_handle's destructor will
correctly clean up partial initialisation.

Remember that the static init function never experiences an exception
throw. It calls 100% noexcept functions. So you write code knowing for a
fact that you don't need to order things for correct exception safety
during unexpected stack unwind. Somehow that's important to my opinion
here, but I don't know why. Maybe when I employ this design pattern,
later on when I revisit this code it immediately signals to me that this
code is 100% never throwing code. So maybe it's a signalling design choice.

> For this reason, I'd think that a style that does
> 
>    file_handle fh(native_handle_type(), 0, 0, std::move(_path),
> _caching, flags);
> 
> first, then initializes it appropriately, and as a final step returns it
> via result<file_handle>, would perhaps make more sense (and also save a
> number of ret.value() calls).

I agree, as I said originally, that most first time users of these
objects would think the same way as you.

But I think after you've been using them for a while, when you're
writing init functions, you'll end up thinking like me. My way is
somehow ... "cleaner". I really can't say why. It just feels right. The
logically more obvious way you spoke of feels wrong, or rather, bad or
dirty somehow.

I wish I could justify myself here. I cannot. I will say that my way
doesn't have RVO problems on C++ 14, yours you need to type extra
non-obvious stuff to some folk. But that's not an argument really.

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/