Oddity with C++ lambda expressions and Octave code

This was a small thing, but kind of amusing about how prickly compilers can be.

@mmuetzel checked in a change in the C++ code for jsonencode.cc to save/restore the value of a warning ID.

Simple enough, he created a file local static function

static void
warning_cleanup (const octave_value_list& args)
  set_warning_state (args);

and then registered this cleanup function with an unwind_protect frame.

octave_value_list ws
  = set_warning_state ("Octave:classdef-to-struct", "off");
octave::unwind_protect frame;
frame.add_fcn (warning_cleanup, ws);

Given that the function was so small, I thought it might be better form to use a lambda expression (anonymous function) so a future programmer could quickly see what behavior was being registered with the unwind_protect frame rather than having to jump to the beginning of the file.

Naively, I thought I could just do this

frame.add_fcn ([] (auto ws_orig) { set_warning_state (ws_orig); }, ws); 

A bunch of things are wrong with this, but the one I first came across is that the lambda notation [] creates a lambda object type and frame.add_fcn() is expecting a function pointer. But lambda objects and pointers to functions are very closely related and so you can use the ‘+’ character to tell the compiler that you want a function pointer. So my next iteration was this

frame.add_fcn (+[] (auto ws_orig) { set_warning_state (ws_orig); }, ws); 

And here is where it got more amusing, the C++ compiler then errored out with hundreds of warnings from liboctave about no function prototype matching addition between an Octave Array object and whatever the compiler was determing for the type of the lambda expression.

I checked in the final code here https://hg.savannah.gnu.org/hgweb/octave/rev/f4ed4be5d809 which uses an intermediate variable just for the compiler’s benefit.

+      auto cleanup_ptr = +[] (octave_value_list ws_orig)
+                           { set_warning_state (ws_orig); } ;
+      frame.add_fcn (cleanup_ptr, ws);

Overall, I’m still not deterred and would like to use modern C++ techniques, but this was annoying.

1 Like

Interesting read. Thank you for that.

I’m still having trouble to understand why this works.

I don’t have access to the C++11 ISO standardization documents. But I found this which seems to be a draft for C++20. (I hope it is not too different to C++11 in this respect):

The closure type for a non-generic lambda-expression with no lambda-capture whose constraints (if any) are satisfied has a conversion function to pointer to function with C++ language linkage having the same parameter and return types as the closure type’s function call operator.
The conversion is to “pointer to noexcept function” if the function call operator has a non-throwing exception specification.
The value returned by this conversion function is the address of a function F that, when invoked, has the same effect as invoking the closure type’s function call operator on a default-constructed instance of the closure type.
F is a constexpr function if the function call operator is a constexpr function and is an immediate function if the function call operator is an immediate function.

So there is a conversion function from lambda expressions of that type (no closure) to good ol’ function pointers.
What I don’t understand is why the unary plus triggers that conversion with the auto return type. Is this compiler specific behavior?
I didn’t find this documented. But maybe I didn’t use the correct search words.

If it is undocumented behavior, would it be better to not rely on it. E.g.:

void (*cleanup_ptr) (const octave_value_list&) =
  [] (octave_value_list ws_orig) { set_warning_state (ws_orig); }

Or maybe also:

void (*cleanup_ptr) (auto) =
  [] (octave_value_list ws_orig) { set_warning_state (ws_orig); }

I haven’t tested those expressions. So I don’t know if they will work.

Maybe even easier for the future and clearer would be if octave::unwind_protect::add_fcn (i.e. its parent class octave::action_container) had an overload for lambda expressions.
Maybe à la:

    template <typename Lambda, typename... Args>
    void add_fcn (Lambda lambda, Args&&... args)
      add_action (new fcn_elem (lambda, std::forward<Args> (args)...));

That way the approach you intuitively took initially would probably have worked out of the box.

I’ll test if that works for me.

The specifications are always so obtuse that they leave one more confused, rather than resolving the issue.

I’m not sure if ‘+’ is in the standard or not, although gcc seems to understand it and a Stack Overflow article I read mentioned it. The problem with not using ‘+’ and describing the type fully is that the code then becomes very long. A big part of the desire for making this change was to make it smaller and easier to understand. The use of the auto keyword helps a lot in that regard.

I think the suggestion to add a new function overload for add_fcn could be the best. Ideally, I would like to write the anonymous function on a single line and take advantage of lambda captures rather than passing an argument to the anonymous function. If we could get add_fcn to accept a lambda then the original code could be written

frame.add_fcn ([&ws] () { set_warning_state (ws); });

which is very efficient.

There is this forum entry that suggests that the unary plus before a lambda expression doesn’t work with MS compilers:

The link to the bug report in the accepted answer is dead. But it looks like at least one compiler on at least one platform doesn’t understand that syntax. (And that seems to be by design.)
Even if we don’t support the Visual Studio compilers, maybe we still should avoid that syntax.

The lambda template seems to work for me. I’ll prepare a changeset and will upload it to savannah.

I recommend that you use an unwind_action object instead of unwind_protect. The unwind_action object can only perform one cleanup action, but it can already handle std::function objects and lambda expressions. Using that object is also more efficient than unwind_protect because the unwind_protect object must instantiate a std::stack object to hold the list of actions.

Separately, we might want to deprecate the unwind_protect object? It originally made sense because there was a single global stack of actions and a set of functions to operate on that stack. But then Jaroslav converted it to use the present structure in which the actions are managed on a per-scope basis using the destructor of the unwind_protect object. But we only ever need to protect a few things (at most) in a given scope, so the overhead of creating the std::stack object can be significant. In nearly all cases, it probably makes more sense to just use a set of unwind_action variables instead of a single unwind_protect object.

While looking at replacing calls to frame.protect_var with unwind_protect_var objects, I was reminded that it is already possible to write things like the following (this is taken from pt-eval.cc):

    frame.add ([this] (void)
                 delete m_debugger_stack.top ();
                 m_debugger_stack.pop ();

Is this what you were originally looking for? If so, sorry it was not obvious.