HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Macro enabling Python style 'with' in C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withstylepythonmacroenabling

Problem

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(my_mutex);
    // oooops, forgot to give the lock a name
    function_call();
}

void bug2() {
    std::unique_lock 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 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
#include

// 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(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 {
n

Solution

[ 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 
std::true_type make_true(T&&, std::false_type) {
    static_assert(std::is_move_constructible::value,
                  "BOOST_WITH requires the scoped object's type to be move"
                  " constructible");
    return std::true_type{};  // never executed
}

template 
always_true make_true(T&& what, std::true_type) {
    static_assert(std::is_move_constructible::value, "this is a bug");
    return always_true{std::forward(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{}))


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; std::false_type = std::integral_constant]’:
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::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.

Code Snippets

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)};
}
#define BOOST_WITH(RESOURCE)                                        \
  if (auto BOOST_WITH_always_true = boost::with_detail::make_true(  \
      RESOURCE,                                                     \
      std::is_move_constructible<decltype(RESOURCE)>{}))

Context

StackExchange Code Review Q#86537, answer score: 6

Revisions (0)

No revisions yet.