boostorg / leaf

Lightweight Error Augmentation Framework
Boost Software License 1.0
302 stars 48 forks source link

BOOST_LEAF_CHECK and extra semicolons #17

Closed mikezackles closed 3 years ago

mikezackles commented 3 years ago

BOOST_LEAF_ASSIGN has no enclosing block, but BOOST_LEAF_CHECK does. This seems inconsistent and means that BOOST_LEAF_CHECK(mycoolthing()); results in an extra semicolon (and potentially a warning).

zajo commented 3 years ago

Are you thinking

#define BOOST_LEAF_CHECK(r)\
    static_assert(::boost::leaf::is_result_type<typename std::decay<decltype(r)>::type>::value, "BOOST_LEAF_CHECK requires a result type");\
    auto && BOOST_LEAF_TMP = r;\
    if( BOOST_LEAF_TMP )\
        ;\
    else\
        return BOOST_LEAF_TMP.error()

?

mikezackles commented 3 years ago

Yes, something along those lines, I think.

I had

#define LEAF_CHECK(r)\
  static_assert(::boost::leaf::is_result_type<typename std::decay<decltype(r)>::type>::value, "BOOST_LEAF_CHECK requires a result type");\
  auto &&BOOST_LEAF_TMP = r;\
  if (!BOOST_LEAF_TMP) return BOOST_LEAF_TMP.error()
zajo commented 3 years ago

Thats not safe in a macro, the if is lacking an else.

mikezackles commented 3 years ago

Makes sense!

mikezackles commented 3 years ago

Actually, I spoke too soon. My macro-fu is weak. Would you just be worried about someone abusing it like LEAF_CHECK(...); else ...?

The one you proposed would be fine for me, if you think that one's acceptable.

zajo commented 3 years ago

Yes, exactly -- it is a very bad idea to leave an incomplete if statement in a macro. This was the main motivation for adding the scope actually, but it's true that it better without the scope, it would allow you to look at the temp variable during debugging.

mikezackles commented 3 years ago

I see. Interesting to know the history, and thanks for the fix!