boostorg / scope

Boost.Scope, a collection of scope guard utilities.

Boost.Scope

Boost.Scope provides a number of scope guard utilities and a unique_resource wrapper, similar to those described in C++ Extensions for Library Fundamentals, Version 3, Section 3.3 Scope guard support [scopeguard]. The implementation also provides a few non-standard extensions.

Directories

• doc - QuickBook documentation sources
• include - Interface headers of Boost.Scope
• test - Boost.Scope unit tests

More information

• Read the documentation.
• Report bugs. Be sure to mention Boost version, platform and compiler you're using. A small compilable code sample to reproduce the problem is always good as well.
• Submit your patches as pull requests against develop branch. Note that by submitting patches you agree to license your modifications under the Boost Software License, Version 1.0.

Build status

Branch	GitHub Actions	Test Matrix	Dependencies
master
develop

License

Distributed under the Boost Software License, Version 1.0.

scope's Issues

Change `unique_resource` example in the documentation

From Julien Blanc:

Another issue raises when we look at the example code:

void write_string(std::string const& filename, std::string const& str)
{
    // Create a unique resource for a file
    boost::scope::unique_resource< int, fd_deleter > file(open(filename.c_str(), O_CREAT | O_WRONLY));
    if (file.get() < 0)
    {
        int err = errno;
        throw std::system_error(err, std::generic_category(), "Failed to open file " + filename);
    }

    // Use it to write data to the file
    const char* p = str.data();
    std::size_t size_left = str.size();
    while (size_left > 0)
    {
        ssize_t written = write(file.get(), p, size_left);
        if (written < 0)
        {
            int err = errno;
            if (err == EINTR)
                continue;
            throw std::system_error(err, std::generic_category(), "Failed to write data");
        }

        p += written;
        size_left -= written;
    }
}

In case of a failure, what we got is an half-baked file, which we know nothing about. A proper write_string function should at least try to do its cleanup and delete the file in case of error. But that defeats the purpose of unique_resource... In my experience, automatically closing file handles is not error handling. But it has its use cases, maybe it would be better demonstrated by an example like:

bool compare_files(std::string const& filename1, std::string const& filename2)
{
    boost::scope::unique_resource< int, fd_deleter > file1(open(filename.c_str(), O_CREAT | O_WRONLY));
    if (file1.get() < 0)
    {
        int err = errno;
        throw std::system_error(err, std::generic_category(), "Failed to open file " + filename1);
    }
    boost::scope::unique_resource< int, fd_deleter > file2(open(filename.c_str(), O_CREAT | O_WRONLY));
    if (file2.get() < 0)
    {
        int err = errno;
        throw std::system_error(err, std::generic_category(), "Failed to open file " + filename2);
    }
    // do the comparison
}

Passing a function reference to a scope guard factory fails to compile

Reported on the boost-dev ML.

Given

void f();

int main()
{
    auto a = boost::scope::make_scope_fail(f);
    boost::scope::scope_fail b(f);
}

Only the declaration of b compiles. The one of a fails. I think the one of a should compile, too. Something like std::decay instead of typename std::remove_cv< typename std::remove_reference< F >::type >::type at https://github.com/Lastique/scope/blob/develop/include/boost/scope/scope_exit.hpp#L535

Prohibit default construction of function pointers

Where default construction of function objects is allowed in the library, pointers to functions must be excluded because default- and value-constructed function pointers are not callable.

Add `BOOST_SCOPE_DETAIL_DOC_HIDDEN` to `unique_resource::reset`

Taking a look at the documentation as well as the source code of unique_resource::reset, I recognized that the macro BOOST_SCOPE_DETAIL_DOC_HIDDEN is probably missing around the noexcept operator. Looking at the source code, the same might be true for the destructor of unique_resource, but this at least does not seem to have impact on the documentation. In the documentation, the destructor of unique_resource doesn't have any noexcept specifier at all.

Why detail::compact_storage?

I forgot to ask this before the review, so I'm asking now: why do you have your own helper that employs EBO rather than using boost::empty_value from Core?

Possible double-move of resource on `unique_resource` construction

From Alexander Grund:

And a possible bug I've just seen: template< typename R, typename D,...> explicit unique_resource_data(R&& res, D&& del)... : unique_resource_data(static_cast< R&& >(res), static_cast< D&& >(del), traits_type::is_allocated(static_cast< R&& >(res))) { }

The static_cast<R&&> look wrong. Should that be a std::move or std::forward? It is std::forward, but I had to check to be sure. (see above where a similar cast was a std::move) To me it looks like itis_allocated should not have a cast, should it? What if it is implemented to accept an instance, you castit to an rvalue, i.e. move it into that call and then use the moved-from value to construct unique_resource_data Similar for the deleter where using std::forward makes the intent clear.

How can I wrap iconv_t in boost::scope::unique_resource in just one line?

I try to do this:

using unique_iconv_t = boost::scope::unique_resource<iconv_t, boost::core::functor<iconv_close>, boost::scope::unallocated_resource<(iconv_t)(-1)>>;
unique_iconv_t cd(iconv_open("WCHAR_T//IGNORE", "UTF8"));

But the compiler gives:

error: ‘-1’ is not a valid template argument for ‘void*’ because it is not the address of a variable

Must I define my own trait class to solve this? Is there a simpler way to write this?

Add support for default construction to scope guards

From Julien Blanc:

scope_exit is not default constructible. But it can be released and is movable, so a default constructed scope_exit state makes sense (this is the same as a moved-from scope_exit). I find this a rather strange design choice. What is the rationale behind this?

I think it does not hurt for scope_fail and scope_exit. We have scope_final for the simple case. I'm asking because i finally added it to the scope_exit-like we use in our code base.

make_default returns wrong type in example

https://github.com/Lastique/scope/blob/develop/doc/unique_resource.qbk#L260 that should probably be int.

Add a docs section discussing differences with Library Fundamentals TS

From Peter Dimov:

You have a section in the documentation that compares Scope to ScopeExit,
but I don't see a comparison against the proposed standard components.

It might be useful to have one, where the rationale for the additions could
naturally fit.

It might be useful to have a section that describes how Boost.Scope components are different from the Library Fundamentals TS and what advantages this brings.

Example in Introduction doesn't return a value on success

https://github.com/Lastique/scope/blob/develop/doc/scope.qbk#L268

The function doesn't return a value on success.

Possible alias for C++-17 unique_resource

One review suggested a different design for unique_resource, which puts default resource and deleter into template parameters. I've realised this can be easily replicated with an alias and a few helper templates: https://godbolt.org/z/aMToasrG8

Add a new set of examples in the Introduction section in the docs

From Andrzej Krzemienski:

Given that in Boost.ScopeExit I need to implement the conditional
execution manually, I can do it in two ways. One is:

template< typename T >
void add_object(T const& arg)
{
    auto it = objects.insert(Object());

    auto uncaught_count = boost::core::uncaught_exceptions();
        
    BOOST_SCOPE_EXIT_TPL(this_, it, uncaught_count)
    {
        if (uncaught_count != boost::core::uncaught_exceptions())
            this_->objects.erase(it);
    }
    BOOST_SCOPE_EXIT_END;

    it->start(arg);
}

The other is:

template< typename T >
void add_object(T const& arg)
{
    auto it = objects.insert(Object());

    bool active = true;

    BOOST_SCOPE_EXIT_TPL(this_, it, &active)
    {
        if (active)
            this_->objects.erase(it);
    }
    BOOST_SCOPE_EXIT_END;

    it->start(arg);
    active = false;
}

Given these options, I have zero motivation for using the former. The
latter one:

• is C++11 compatible (boost::uncaught_exceptions doesn't necessarily
  work for prec++17 compilers)
• works correctly in 100% cases rather than in 99% of the cases (I mean
  the caveats regarding coroutines and fibers)
• avoids two calls to a C++ runtime.

I think the example with the active flag should be used in comparison.
And then, if you want to compare apples to apples, you would have to use
scope_exit and set_active. Otherwise, I think the comparison should
acknowledge that the solution with scope_fail has caveats and is slower.

We've discussed this before, and there are pros and cons to each
approach. The uncaught_exceptions-based solution encapsulates the
rollback logic in one place, you don't have to set the active flag
manually on every return point. It does have downsides as well, like
higher performance cost (unless the compiler implements it using an
intrinsic) and incompatibility with coroutines (which might not matter
if you don't use or care about coroutines). So neither one is wrong or
bad, it all depends on your requirements. Personally, I have used the
solution based on uncaught_exceptions quite a few times back when I was
using Boost.ScopeExit, for the reasons I mentioned.

Back to the table in the docs, there are two main points I'd like to
make. First, this is an Introduction section. It is not meant to be an
exhaustive description of library features and caveats. It only serves
to give the reader the basic understanding of what the library is about
and a glimpse of how it might look in his code. Hence I do not think
that it is the right place to dive into details and corner cases, there
are other sections in the docs for that.

Second, the reason I'm giving the uncaught_exceptions-based example in
the table, besides that this is the kind of code I actually wrote and
eventually converted to Boost.Scope, is that the
uncaught_exceptions-based version of the Boost.ScopeExit code is the
exact equivalent of the Boost.Scope example that uses scope_fail. IOW,
this is apples-to-apples comparison, as opposed to using different
solutions for different libraries. I think, this is only fair.

You may argue to include a row of examples, where Boost.ScopeExit code
uses the active flag. I suppose, I could do this, although to a great
degree it would duplicate the row that already exists. I'm not sure how
useful it would be, as it would considerably increase the size of the
section for little value.

I would encourage you to add a third row.
I understand that "Introduction" is only to give a potential user the first impression, and this is exactly why I am pushing for this. The current presentation has two downsides:

• It presents Boost.ScopeExit as worse than it really is.
• It might give an impression that the author is unaware of coroutine/fiber-related problems, and offers no solution for this case.

Docs: usage of `activate` in the initial example

The initial example in the docs uses:

it->activate(arg);

This is a bit confusing, given that guards have a member function set_active, one starts to wonder if it points to a guard. Any other name, like start, would be better.

Anyway, is this a real life example? Wouldn't you rather create an automatic object, activate (or start) it there, and then move into the set? This way, you do not need a guard.

Put safety checks in exception_checker

In the destructor of excepiton checker
https://github.com/Lastique/scope/blob/develop/include/boost/scope/exception_checker.hpp#L81
put an assertion:

    result_type operator()() const noexcept
    {
        int new_uncaugth_count = boost::core::uncaught_exceptions();
        BOOST_ASSERT(new_uncaugth_count  == m_uncaught_count || new_uncaugth_count == m_uncaught_count + 1);
        return new_uncaugth_count > m_uncaught_count;
    }

This will help detect situations (even if not all) when excepiton checker is used in a coroutine or some other suspendable task.

Wrong description of what can be passed to a guard

https://github.com/Lastique/scope/blob/2cd37e97c91fb1e76bdb8d33464cf89ea42923be/doc/scope_guards.qbk#L17 says:

The wrapped action function object is specified on the scope guard construction and cannot be changed afterwards. It must be one of:

• a user-defined class with a public operator() taking no arguments, or
• an lvalue reference to such class, or
• an lvalue reference or pointer to a function taking no arguments.

There is something wrong with this description. Are you describing types that are allowed or objects that are allowed?