Macro enabling Python style 'with' in C++

Question

I am developing a simple macro that enables Python style 'with' in C++. I have shamelessly prefixed the macro name with BOOST_ - primarily because there's a similarity to BOOST_FOREACH, and because I didn't want to call it plain WITH.

The motivation are familiar bugs like these:

void bug1() {
    std::unique_lock<std::mutex>(my_mutex);
    // oooops, forgot to give the lock a name
    function_call();
}

void bug2() {
    std::unique_lock<std::mutex> waldo(my_mutex);
    function_call();
    // oooops, forgot to unlock
    other_function_call();
}

void ugly() {
    {
        // intent of explicit scope not as obvious as could be
        std::unique_lock<std::mutex> waldo(my_mutex);
        function_call();
    }
    other_function_call();
}

So here are the questions:

• Is this useful to anyone but me?
• Is this a bad idea for whatever reason?
• Have I overlooked a possibility to implement this in C++03?
• Can it be implemented without the is_move_constructible type requirement?
• Do you have suggestions for improvement?
• (With a twinkle) Should I apply for some speaking time during the C++Now 2015 Lightning Talks and present this?

And here's my implementation:

NOTE: I have considered an alternative signature such as...

BOOST_WITH(type, ...)

...where ... would be the arguments to the constructor. I have decided to stick to this:

#ifndef BOOST_WITH_HPP_INCLUDED
#define BOOST_WITH_HPP_INCLUDED

#include <utility>
#include <type_traits>

// This macro expands to code that can be used in the same way as the standard
// control structures can. Whatever 'exp' returns lives as long as a loop
// variable would in similar context.
//
// Example:
// BOOST_WITH(std::unique_lock<std::mutex>(my_mutex))
//     do_something();
//
// Example:
// BOOST_WITH(Pushed_matrix()) {
//     draw_something();
//     draw_something_else();
// }
#define BOOST_WITH(exp)                                                        \
    if (auto BOOST_WITH_always_true = boost::with_detail::make_true(exp))


namespace boost {
namespace with_detail {

// wraps an object of movable type
// and provides conversion to bool (always true)
template <class T>
struct always_true {
    explicit always_true(T what) : x{std::move(what)} {}
    constexpr operator bool() const { return true; }
    T x;
};

// always_true<T> construction helper
template <class T>
always_true<T> make_true(T&& what) {
    static_assert(std::is_move_constructible<T>::value,
                  "BOOST_WITH requires the scoped object's type to be move "
                  "constructible");
    return always_true<T>{std::forward<T>(what)};
}

}} // namepace boost::with_detail


#endif // BOOST_WITH_HPP_INCLUDED

Finally, a minimal example:

#include "with.hpp"
#include <cassert>
#include <mutex>

// nothing serious, just a prove of concept
struct Lockable {
    Lockable() : locked(false) {}
    void lock() { locked = true; }
    void unlock() { locked = false; }
    bool locked;
};

int main() {
    Lockable the_lock;
    assert(!the_lock.locked);
    BOOST_WITH(std::unique_lock<Lockable>(the_lock))
        assert(the_lock.locked);
    assert(!the_lock.locked);
}

Answer 1

[ Foreword: Note that I'm not going to argue whether or not it is a good idea to invent a new macro-based syntax for this. I'm only reviewing your code. — end foreword ]

I think you have been pretty careful and cannot spot any serious issues with your implementation; nice job.

The only thing that I think could be improved is the error reporting. Your static_assertion fires much too late in the instantiation process such that the error message will end up buried under loads of template error messages. You can re-structure the instantiation machinery to improve this.

First, provide two overloads of boost::with_detail::make_true:

template <class T>
std::true_type make_true(T&&, std::false_type) {
    static_assert(std::is_move_constructible<T>::value,
                  "BOOST_WITH requires the scoped object's type to be move"
                  " constructible");
    return std::true_type{};  // never executed
}

template <class T>
always_true<T> make_true(T&& what, std::true_type) {
    static_assert(std::is_move_constructible<T>::value, "this is a bug");
    return always_true<T>{std::forward<T>(what)};
}

The static_assertion in the second overload is of course only paranoia and could be omitted.

Second, select the appropriate overload via tag dispatch:

#define BOOST_WITH(RESOURCE)                                        \
  if (auto BOOST_WITH_always_true = boost::with_detail::make_true(  \
      RESOURCE,                                                     \
      std::is_move_constructible<decltype(RESOURCE)>{}))

With this change, if I try to abuse your macro with a wrong type, GCC gives me this pretty straight-forward error message:

In file included from main.cpp:1:0:
boost/with.hpp: In instantiation of ‘std::true_type boost::with_detail::make_true(T&&, std::false_type) [with T = std::mutex; std::true_type = std::integral_constant<bool, true>; std::false_type = std::integral_constant<bool, false>]’:
main.cpp:22:5:   required from here
boost/with.hpp:42:7: error: static assertion failed: BOOST_WITH requires the scoped object's type to be move constructible
       static_assert(std::is_move_constructible<T>::value,

Compare this with what we would have got before.

Note that we cannot simply put static_assert(false, …); inside the make_true body because it would fire unconditionally on every compilation. It is necessary to make the asserted condition dependent on the template argument. Of course, any other template instantiated with T that always yields false would do equally well but using std::is_move_constructible (again) seems easiest.

Answer 2

Abstraction in C++ is not a bad thing, neither does making the code more fluent, readable and (oh my) even fun!

my only answer for your question is : do we really need to imitate different languages inside our code when C++ already has what we need?

template <class retType , class... Args>
retType ensureSynchronized (retType(*givenFunction)(Args...) , Args... args){
         std::lock_guard<std::mutex> synchLock (somePredefinedMutex);
         return givenFunction(std::forward<Args>(args)...);
}

example:

int main(void){

try{
   ensuereSynchronized (myFunc, 1 ,ofstream("myFile"),someEnum::someValue);
}
catch{
  //cleanups
}
return 0;
}

isn't it cleaner , simpler AND solve all of your bugs? (plus does not use boost but the standard)

Answer 3

So here are the questions:

•Is this useful to anyone but me?

I wouldn't use it; It is basically copying/generalizing the functionality of a scoped lock (alter another object's state, while in scope) - and I would prefer a simple RAII wrapper for that (because it's expected, straightforward and doesn't write code using macros).

•Is this a bad idea for whatever reason?

Writing code using macros is usually a brittle solution (that is, "it works and it's fine" - as long as the code needs no maintenance).

The original code (the examples you give of symptomatic errors) should be fixed (in my opinion) according to SRP.

Your example (with my comments documenting my assumptions):

void bug2() {
    std::unique_lock<std::mutex> waldo(my_mutex); // scoped lock
    function_call();                              // should be synchronized
    other_function_call();                        // should NOT be synchronized
}

Conceptually, bug2 performs two types of operations (operates at different abstraction levels):

• handle synchronization of function_call
• perform sequence of two function calls

If you extract the first operation to a dedicated function:

// perform safe "function_call()"
// and _do nothing else_
void safe_function_call()
{
    std::unique_lock<std::mutex> waldo(my_mutex); // scoped lock
    function_call();                              // synchronized 
}

void bug2() // note: rename function, as it is no longer a bug
{
    safe_function_call(); // synchronized call is now an atomic operation
                          // as far as client code is concerned
    other_function_call();
}

This provides a better abstraction level separation and better primitives in your client code.

If you hide the existence of function_call in a separate compilation unit or protected/private area of a class, you create a situation where the original bug can no longer be replicated (anywhere) because you only allow for the safe alternative.

Answer 4

If anyone else finds this, such a macro can be greatly simplified in C++17:

#define WITH(exp) if ([[maybe_unused]] auto WithVarName = exp; true)