pt., 1 gru 2023 o 19:49 Andrey Semashev via Boost napisał(a):

On 12/1/23 20:25, Andrzej Krzemienski via Boost wrote:

...

Hi All, This is not a review yet. I am just looking at the front page of the docs, and I think the comparison with Boost.ScopeGuard is unfair.

You are comparing C++03 Boost.ScopeGuard with C++11 Boost.Scope. Try comparing it with C++03 Boost.Scope.

Boost.Scope doesn't support C++03.

I'm comparing the libraries using the syntax they provide, regardless of the C++ version, when they both provide equivalent functionality.

...

Otherwise compare C++11 to C++11, and there Boost.ScopeGuard has a nicer syntax:

int compute() { // Reset variables on return BOOST_SCOPE_EXIT(this_) { this_->x = 0; this_->y = 0; };

return x + y; }

I don't see where the documentation makes BOOST_SCOPE_EXIT_END optional. In every example I can see, BOOST_SCOPE_EXIT is accompanied by BOOST_SCOPE_EXIT_END, C++11 or not. Am I missing something?

Perhaps, you are defining BOOST_SCOPE_EXIT_CONFIG_USE_LAMBDAS and that allows to omit BOOST_SCOPE_EXIT_END? In that case, there is a discussion at the end of this section:

https://lastique.github.io/scope/libs/scope/doc/html/scope/scope_guards.html...

that pertains to your use case, and that makes Boost.ScopeExit very much not equivalent to Boost.Scope.

Thank you. I stand corrected.

...

Regarding the example with exceptions, nobody would use Boost.ScopeGuard like this. A robust usage is in Boost.ScopeGuard's docs:

void world::add_person(person const& a_person) { bool commit = false;

persons_.push_back(a_person); // (1) direct action // Following block is executed when the enclosing scope exits. BOOST_SCOPE_EXIT(&commit, &persons_) { if(!commit) persons_.pop_back(); // (2) rollback action };

// ... // (3) other operations

commit = true; // (4) disable rollback actions

}

Which is longer, but usable in coroutines. Whereas Boost.Scope's solution is not usable in coroutines.

First, what example are you referring to?

The second (and the last) row of the same table in the Introduction section.

Second, if you mean incompatibility of exception_checker with coroutines then that is already addressed in a note in this section:

https://lastique.github.io/scope/libs/scope/doc/html/scope/scope_guards.html...

If you're using coroutines then just don't use it. Where it is used in library examples, it is implied that coroutines are not used.

I mean something slightly different. 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. Regards, &rzej;

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

sob., 2 gru 2023 o 17:11 Andrey Semashev via Boost napisał(a):

On 12/2/23 17:14, Andrzej Krzemienski wrote:

...

pt., 1 gru 2023 o 19:49 Andrey Semashev via Boost

Second, if you mean incompatibility of exception_checker with coroutines then that is already addressed in a note in this section:

https://lastique.github.io/scope/libs/scope/doc/html/scope/scope_guards.html... < https://lastique.github.io/scope/libs/scope/doc/html/scope/scope_guards.html...

...

If you're using coroutines then just don't use it. Where it is used

in

...

library examples, it is implied that coroutines are not used.

I mean something slightly different.

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. Regards, &rzej;