Call a lua function - the C++ way

Question

I'm not that good when it comes to templates, especially when it comes to variadic templates. I want to show you some code where I use both - templates and variadic templates - to call a lua function. I'm proud that it works, and it works great, but I want to get some feedback on what I could've done better.

template<typename ... Args>
bool run_script(int id, Args && ... args) {
 const int params = sizeof...(args);
 _load(id);
 push(std::forward<Args>(args)...);
 return _run(params);
}

This is the "entry point". _load will just load the lua-function onto the stack and _run will run it with params arguments. The important part is the push function.

template<typename T>
void push(T && arg) {
 _push(std::forward<T>(arg));
}
template<typename T, typename ... Args>
void push(T && arg, Args && ... other) {
 _push(std::forward<T>(arg));
 push(std::forward<Args>(other)...);
}
void _push(int arg) {
 lua_pushinteger(L, arg);
}
void _push(const std::string& arg) {
 lua_pushstring(L, arg.c_str());
}
void _push(const char* arg) {
 lua_pushstring(L, arg);
}

I had a problem before, where I had all of them named push, but for some reason it couldn't push a std::string (std::string as argument choose the template instead of const std::string&) but now with the templates called push and the functions which do the work _push it's working great. It's not much code, I know, but it's my the first time working with variadic templates and it took me a fair amount of time to get it working.

• Arahman: Is std::forward used to avoid unnecessary copy operations or is there some other reason std::forward should be used?

Answer 1

As your code goes everything i believe is fine. Define the template, define overloads and combine. Some things i've noticed that might be arguable:

1. Is it possible that your script will have zero arguments? For example a script with id = N denotes print last command or show commands history. If yes, your code wont compile since

   template<typename T> 
   void push(T && arg) {
    _push(std::forward<T>(arg));
   }
   

   doesnt accept zero arguments. Perhaps replace it with zero arg overload?

   template<typename T>
   void push() {
    // cout << "All parameters passed to Lua! [Empty args case]\n"; 
   }
   

2. Also as Args && ... other can actually be zero length there's a question of which will be called template<typename T> void push(T && arg) or template<typename T, typename ... Args> void push(T && arg, Args && ... other). Since push(std::forward<Args>(other)...) can not compile with 0 args, then the first overload will get called. I'd call that a bit misleading if somebody else has to work with your code.

3. The run_script func is packed with expressions which could potentially fail. So i'd probably try to encapsulate each step into its own method and move the _run() to the zero template push.

   template<typename T>
   void push() {
    // cout << "All parameters passed to Lua! [Empty args case]\n";
    _run(params); 
   }
   

   That way if push<T>() gets called you're sure all the parameters were passed to interpreter.

4. Question arises here is how do you pass params to zero arg push? The topic says C++ way and that means OOP. Imo if you're asking about the C++ style then perhaps you should encapsulate your logic and provide an interface to the user. So go for a class which would hold the arg number as a private member and handle the actual processing of different types. Perhaps two classes one for command and one for processing it. That way it will be easier to scale.
   And before doing that i'd think about the actual use of your application:

   • Will somebody else will be using it?
   • Will it be used in a multithreaded environment?
   • What guarantees for the user do you want to provide?

5. Your entry function bool run_script is supposed to return true or false i guess depending on how the execution of the script went. Does it really boil down to two outcomes? Are you giving a guarantee that the code would execute no matter what? Perhaps a enum class would suit better.

6. Your lua_pushstring(L, arg) and lua_pushinteger(L, arg) all hold some L - so seems only fair to hide it somewhere in the class as a private member as well as arg num.

7. Are you sure that you'll know all your types at compile time? What if somebody passes a double? There are more elegant ways to check the type being passed but this will do:

   template <class A, class B>
   struct eq {
    static const bool result = false;
   };
   template<class A> struct eq<A, A> {
    static const bool result = true;
   };
   

   and use

   if (!eq<int, A>::result) {
    //...unsupported type error
   } 
   

p.s. great project btw. A while a go I've used sockets in Lua and C++ to communicate. So if your code is online I'd love to check it out.

• Incomputable: you could use std::is_same<T, U>::value or wrap your own template that checks if the type is in the allowed types list. Probably the latter will be needed. In C++17, it will probably be much easier with native Concepts support.
• Snowhawk: Concepts didn't make it for C++17.

Answer 2

template<typename ... Args>
bool run_script(int id, Args && ... args) {
 const int params = sizeof...(args);
 _load(id);
 push(std::forward<Args>(args)...);
 return _run(params);
}

Make sure you are not overflowing the lua stack. From the Lua 5.3 Manual,

4.2 – Stack Size

When you interact with the Lua API, you are responsible for ensuring consistency. In particular, you are responsible for controlling stack overflow. You can use the function lua_checkstack to ensure that the stack has enough space for pushing new elements.

Whenever Lua calls C, it ensures that the stack has space for at least LUA_MINSTACK extra slots. LUA_MINSTACK is defined as 20, so that usually you do not have to worry about stack space unless your code has loops pushing elements onto the stack.

When you call a Lua function without a fixed number of results (see lua_call), Lua ensures that the stack has enough space for all results, but it does not ensure any extra space. So, before pushing anything in the stack after such a call you should use lua_checkstack.

---

For functions prefixed with _, are these meant to be apart of the public interface? If not, hide them through a named namespace (impl, detail) to indicate they shouldn't be called.

---

Each of your _push functions works only with the hard-coded L lua_state*. Consider refactoring the state into an argument.

Ensure you are using a resource manager for L (the lua_state) so that it doesn't leak. Use the C++ type system until you have to interact at a boundary (like a C-API).

---

When recursively pack expanding, you have to consider what can be passed. You handled the cases where the pack contains arguments. What happens if the pack doesn't contain arguments? As @prettyfly mentioned, define the behavior for that case.

Looking at "Parameter Pack", we can also expand through a braced-init-list ({}). A braced-init-list guarantees left-to-right evaluation of the parameter pack.

Note - The reference page for "Parameter Pack" uses int dummy[sizeof...(Ts)] for the pack expansion. Zero-length arrays are not standard and are only supported through compiler extensions. Ensure the resulting expansion has a size of 1 by padding the resulting array. - end note

namespace detail {
 void push(LuaState& state, int arg) {
 lua_pushinteger(*state, arg);
 }
 // other forwarded types to push
}
template <typename Arg>
void push(LuaState& state, Arg&& arg) {
 return detail::push(state, std::forward<Arg>(arg));
}
template <typename... Args>
void push_all(LuaState& state, Args&&... args) {
 std::initializer_list<int>{(push(state, std::forward<Args>(args)), 0)...};
}

If you pass zero arguments, nothing is evaluated. If you pass in arguments, they are each forwarded to the single argument helper. You should be aware of the following with this method:

1. braced-init-list expansion will produce

   warning: expression result unused [-Wunused-value]

   To discard the expression result, §5.2.9/4 in the C++ Standard allows us to cast the result to void.

   (void) std::initializer_list<int>{(push(state, std::forward<Args>(args)), 0)...};
   ^^^^^^
   

2. The comma operator is vulnerable to user-defined-types that overload operator,. You can guard against such an evil practice in two ways

   a. Pass void() to operator,

   (void) std::initializer_list<int>{(push(state, std::forward<Args>(args)), void(), 0)...};
    ^^^^^^ 
   

   Functions cannot be defined to take arguments of type void. If a void type is passed as an argument to a template, substitution failure will occur, and the compiler reverts to the built-in comma operator.

   b. Discard the result

   (void) std::initializer_list<int>{((void) push(state, std::forward<Args>(args)), 0)...};
    ^^^^^^
   

In c++17, we'll have fold-expressions, so we'll be able to just write

 ((void)(push(state, args), ...); // or
 (push(state, args), void(), ...);

Each expanded argument still needs to be guarded against user-defined data types overloading operator,.

---

Rather than specializing for every type,

void _push(int arg) {
 lua_pushinteger(L, arg);
}

Consider specializing on constrained types through tag dispatching (tutorial here). Tag dispatching leverages the C++ language and compiler to select the appropriate candidate function through tags. <type_traits> provides the tools you'll need to test for most of the types. For missing types (like is_string<T>), you'll have to roll your own or pull from Boost or another metaprogramming library. There are other metaprogramming techniques that can be used as well until C++ implements the Concepts proposal in a future standard (hopefully in C++20).