4

Recently, I came across an issue about the design of scope guard. A scope guard invokes a supplied function object (usually performs cleanup procedures) upon exiting the enclosing scope. The current design is as follows:

#define HHXX_ON_SCOPE_EXIT_F(...) ...
#define HHXX_ON_SCOPE_EXIT(...) HHXX_ON_SCOPE_EXIT_F([&]() { __VA_ARGS__ })

HHXX_ON_SCOPE_EXIT_F(...) executes the function object as defined by __VA_ARGS__ upon exiting the enclosing scope. For example,

int i = 0;
{
  HHXX_ON_SCOPE_EXIT_F([&]() {
    assert(++i == 4);
  });
  HHXX_ON_SCOPE_EXIT_F([&]() {
    assert(++i == 3);
  });
  HHXX_ON_SCOPE_EXIT(
    assert(++i == 2);
  );
  HHXX_ON_SCOPE_EXIT(
    assert(++i == 1);
  );
}
assert(i == 4);

The implementation is also simple:

#define HHXX_ON_SCOPE_EXIT_F(...)                                              \
  auto HHXX_UNIQUE_NAME(hhxx_scope_guard) =                                    \
    make_scope_guard(__VA_ARGS__)

#define HHXX_ON_SCOPE_EXIT(...) HHXX_ON_SCOPE_EXIT_F(& { VA_ARGS })


template <typename F>
class scope_guard {
public:
  explicit scope_guard(F f)
      : f_(std::move(f)) {
    // nop
  }
  ~scope_guard() {
    f_();
  }


private:
  F f_;
};


template <typename F>
auto make_scope_guard(F f) {
  return scope_guard<F>(std::move(f));
}

As you can see, it doesn't provide built-in support for a method to abandon the invocation at scope exit. Providing such a feature, however, invalidates conciseness and simplicity of the current design. It also induces certain performance overhead. Basically, I'm in favor of maintaining the current design and rejecting the feature proposal. I want to hear opinions from you, so as to be sure I'm making a right decision.

Glorfindel
  • 3,167
Lingxi
  • 155

2 Answers2

5

Your scope guard has interesting behaviour, and a quick code review could find various issues or possible issues (using the macro takes more code than not using the macro; your macro is flawed because it is a macro and is unnecessary; the gensym macro can't be used two times on the same line, which could happen in macros; many types are not move-constructible; many types are not copyable; you make a copy, then move the copy; I cannot lend a reference of a function to the guard because it creates a copy; ideally, the used callback would be nothrow; …).

You have correctly seen that disabling the guard can be trivially implemented by a user. An implementation that wants to be minimal and composable does not have to offer convenience features.

However, convenience features are mightily convenient, and make the difference between a correct implementation, and a useful implementation. Note that if you implement your guard in terms of std::function or in terms of a std::unique_ptr, it is trivial to support disabling the function without additional complication.

The last time I used such a feature was when I fork()ed a process and only wanted the guard to execute in the parent process, e.g. a guard to log a scope exit. Logging is a glorious use of guards.

However, guards are extremely useful in another circumstance: if I only want to handle exceptional cases, e.g. to roll back a transaction if anything goes wrong:

{
  Transaction transaction {};
  scope_guard rollbackGuard([&](){ transaction.rollback(); });
  ... // do things that could go wrong
  rollbackGuard.disarm();
  transaction.commit();
}

(note the use of a guard name to make the code more self documenting. Gensyms suck).

This would typically be written with a try-catch instead:

{
  Transaction transaction {};
  try { ... } // do things that could go wrong
  catch (...) { transaction.rollback(); throw; }
  transaction.commit();
}

Please understand Transaction as a symbol for some computation; if it were a singular object it should of course manage the rollback in its own destructor unless the commit succeeded.

The big advantage of guards over try–catch–finally is that a guard allows me to declare the on-exit code close to the data it interacts with, rather than 100 lines further down in a catch. If you allow your guards to be dismissed, your scope guards not only support the equivalent of try–finally, but also try–catch-with-rethrow. I would strongly suggest you go down this route, since it adds major convenience to users, with only minor impact on your implementation.

amon
  • 135,795
4

Well, there are different types of scope-guards you could create, depending on what features you need:

  1. Can it be disarmed? That one is free, something along those lines has to be implemented anyway. One cannot depend on RVO always coming to the rescue.
  2. Shall it only execute if an exception is thrown? This needs C++17, unfortunately.

In any way, there is already a class fitting the bill: std::unique_ptr.
Really, it just needs a simple convenience-function for ease of use.

For simplicity I used C++14 automatic return-type-deduction:

template<class F>
auto finally(F f) noexcept(noexcept(F(std::move(f)))) {
    auto x = [f = std::move(f)](void*){ f(); };
    return std::unique_ptr<void, decltype(x)>((void*)1, std::move(x));
}

auto guard = finally([&]{ final_action(); });

With C++17, one can restrict it to only execute when an exception got thrown:

template<class F>
[[nodiscard]] auto on_throw(F f) noexcept(noexcept(F(std::move(f)))) {
    auto x = [f = std::move(f)](void* p){
        if((intptr_t)p > std::uncaught_exceptions()) f();
    };
    return std::unique_ptr<void, decltype(x)>(
        (void*)(intptr_t)(std::uncaught_exceptions() + 1), std::move(x));
}

auto rollback = on_throw([&]{ do_rollback(); });

In both cases, disarming is easily done by calling .release().

Using the common extension __COUNTER__ and some preprocessor-tricks as ssuggested by user2176127 in a comment below makes it even more convenient:

#define UNIQUE_ID_1(x) _Unique_Id_##x
#define UNIQUE_ID_0(x) UNIQUE_ID_1(x)
#define UNIQUE_ID UNIQUE_ID_0(__COUNTER__)

#define FINALLY(...) auto UNIQUE_ID = finally([&]{ VA_ARGS; });
#define ROLLBACK(...) auto UNIQUE_ID = finally([&]{ VA_ARGS; });


FINALLY(final_action());
ROLLBACK(do_rollback());
Deduplicator
  • 9,209