MsgPack-RPC dispatch based on function array lookup #864

This simplifies the generated msgpack_rpc_dispatch() function, separates the
code for each RPC method more clearly and allows easy implementation of
alternative dispatching methods (e.g. string method id dispatch).

scripts/msgpack-gen.lua

@@ -108,7 +108,7 @@ const uint8_t msgpack_metadata[] = {
 
 ]])
 -- serialize the API metadata using msgpack and embed into the resulting
--- binary for easy querying by clients 
+-- binary for easy querying by clients
 packed = msgpack.pack(api)
 for i = 1, #packed do
   output:write(string.byte(packed, i)..', ')
@@ -116,38 +116,32 @@ for i = 1, #packed do
     output:write('\n  ')
   end
 end
--- start the dispatch function. number 0 is reserved for querying the metadata,
--- usually it is the first function called by clients.
 output:write([[
 };
 const unsigned int msgpack_metadata_size = sizeof(msgpack_metadata);
 
-Object msgpack_rpc_dispatch(uint64_t channel_id,
-                            uint64_t method_id,
-                            msgpack_object *req,
-                            Error *error)
-{
-  Object ret = NIL;
-  switch (method_id) {
 ]])
 
--- Visit each function metadata to build the case label with code generated
+-- start the handler functions. First handler (method_id=0) is reserved for
--- for validating arguments and calling to the real API
+-- querying the metadata, usually it is the first function called by clients.
+-- Visit each function metadata to build the handler function with code
+-- generated for validating arguments and calling to the real API.
 for i = 1, #api.functions do
   local fn = api.functions[i]
   local args = {}
-  local cleanup_label = 'cleanup_'..i
-  output:write('\n    case '..fn.id..': {')
 
-  output:write('\n      if (req->via.array.ptr[3].via.array.size != '..#fn.parameters..') {')
+  output:write('static Object handle_'..fn.name..'(uint64_t channel_id, msgpack_object *req, Error *error)')
-  output:write('\n        snprintf(error->msg, sizeof(error->msg), "Wrong number of arguments: expecting '..#fn.parameters..' but got %u", req->via.array.ptr[3].via.array.size);')
+  output:write('\n{')
-  output:write('\n        goto '..cleanup_label..';')
+  output:write('\n  if (req->via.array.ptr[3].via.array.size != '..#fn.parameters..') {')
-  output:write('\n      }\n')
+  output:write('\n    snprintf(error->msg, sizeof(error->msg), "Wrong number of arguments: expecting '..#fn.parameters..' but got %u", req->via.array.ptr[3].via.array.size);')
+  output:write('\n    goto cleanup;')
+  output:write('\n  }\n')
+
   -- Declare/initialize variables that will hold converted arguments
   for j = 1, #fn.parameters do
     local param = fn.parameters[j]
     local converted = 'arg_'..j
-    output:write('\n      '..param[1]..' '..converted..' msgpack_rpc_init_'..string.lower(param[1])..';')
+    output:write('\n  '..param[1]..' '..converted..' msgpack_rpc_init_'..string.lower(param[1])..';')
   end
   output:write('\n')
   -- Validation/conversion for each argument
@@ -157,18 +151,17 @@ for i = 1, #api.functions do
     arg = '(req->via.array.ptr[3].via.array.ptr + '..(j - 1)..')'
     converted = 'arg_'..j
     convert_arg = 'msgpack_rpc_to_'..string.lower(param[1])
-    output:write('\n      if (!'..convert_arg..'('..arg..', &'..converted..')) {')
+    output:write('\n  if (!'..convert_arg..'('..arg..', &'..converted..')) {')
-    output:write('\n        snprintf(error->msg, sizeof(error->msg), "Wrong type for argument '..j..', expecting '..param[1]..'");')
+    output:write('\n    snprintf(error->msg, sizeof(error->msg), "Wrong type for argument '..j..', expecting '..param[1]..'");')
-        
+    output:write('\n    error->set = true;')
-    output:write('\n        error->set = true;')
+    output:write('\n    goto cleanup;')
-    output:write('\n        goto '..cleanup_label..';')
+    output:write('\n  }\n')
-    output:write('\n      }\n')
     args[#args + 1] = converted
   end
-  
+
   -- function call
   local call_args = table.concat(args, ', ')
-  output:write('\n      ')
+  output:write('\n  ')
   if fn.return_type ~= 'void' then
     -- has a return value, prefix the call with a declaration
     output:write(fn.return_type..' rv = ')
@@ -196,37 +189,67 @@ for i = 1, #api.functions do
       output:write('error);\n')
     end
     -- and check for the error
-    output:write('\n      if (error->set) {')
+    output:write('\n  if (error->set) {')
-    output:write('\n        goto '..cleanup_label..';')
+    output:write('\n    goto cleanup;')
-    output:write('\n      }\n')
+    output:write('\n  }\n')
   else
     output:write(');\n')
   end
 
   if fn.return_type ~= 'void' then
-    output:write('\n      ret = '..string.upper(fn.return_type)..'_OBJ(rv);')
+    output:write('\n  Object ret = '..string.upper(fn.return_type)..'_OBJ(rv);')
   end
   -- Now generate the cleanup label for freeing memory allocated for the
   -- arguments
-  output:write('\n\n'..cleanup_label..':');
+  output:write('\n\ncleanup:');
 
   for j = 1, #fn.parameters do
     local param = fn.parameters[j]
-    output:write('\n      msgpack_rpc_free_'..string.lower(param[1])..'(arg_'..j..');')
+    output:write('\n  msgpack_rpc_free_'..string.lower(param[1])..'(arg_'..j..');')
+  end
+  if fn.return_type ~= 'void' then
+    output:write('\n  return ret;\n}\n\n');
+  else
+    output:write('\n  return NIL;\n}\n\n');
   end
-  output:write('\n      break;');
-  output:write('\n    };\n');
-
 end
 
 output:write([[
-
+static Object handle_missing_method(uint64_t channel_id,
-
+                                    msgpack_object *req,
-    default:
+                                    Error *error)
-      snprintf(error->msg, sizeof(error->msg), "Invalid function id");
+{
-      error->set = true;
+  snprintf(error->msg, sizeof(error->msg), "Invalid function id");
-  }
+  error->set = true;
-  return ret;
+  return NIL;
 }
+
 ]])
+
+-- Generate the table of handler functions indexed by method id
+output:write([[
+static const rpc_method_handler_fn rpc_method_handlers[] = {
+  [0] = (rpc_method_handler_fn)NULL]])
+
+for i = 1, #api.functions do
+  local fn = api.functions[i]
+  output:write(',\n  ['..i..'] = handle_'..fn.name..'')
+end
+output:write('\n};\n\n')
+
+output:write([[
+Object msgpack_rpc_dispatch(uint64_t channel_id,
+                            uint64_t method_id,
+                            msgpack_object *req,
+                            Error *error)
+{
+]])
+output:write('\n  // method_id=0 is specially handled')
+output:write('\n  assert(method_id > 0);')
+output:write('\n');
+output:write('\n  rpc_method_handler_fn handler = (method_id <= '..#api.functions..') ?')
+output:write('\n    rpc_method_handlers[method_id] : handle_missing_method;')
+output:write('\n  return handler(channel_id, req, error);')
+output:write('\n}\n')
+
 output:close()

src/nvim/os/msgpack_rpc.h

@@ -16,6 +16,12 @@ typedef enum {
   kUnpackResultNeedMore   /// Need more data
 } UnpackResult;
 
+/// The rpc_method_handlers table, used in msgpack_rpc_dispatch(), stores
+/// functions of this type.
+typedef Object (*rpc_method_handler_fn)(uint64_t channel_id,
+                                        msgpack_object *req,
+                                        Error *error);
+
 /// Dispatches to the actual API function after basic payload validation by
 /// `msgpack_rpc_call`. It is responsible for validating/converting arguments
 /// to C types, and converting the return value back to msgpack types.
@@ -25,12 +31,12 @@ typedef enum {
 /// @param channel_id The channel id
 /// @param method_id The method id
 /// @param req The parsed request object
-/// @param err Pointer to error structure
+/// @param error Pointer to error structure
 /// @return Some object
 Object msgpack_rpc_dispatch(uint64_t channel_id,
                             uint64_t method_id,
                             msgpack_object *req,
-                            Error *err)
+                            Error *error)
   FUNC_ATTR_NONNULL_ARG(2) FUNC_ATTR_NONNULL_ARG(3);
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
@@ -38,4 +44,3 @@ Object msgpack_rpc_dispatch(uint64_t channel_id,
 #endif
 
 #endif  // NVIM_OS_MSGPACK_RPC_H
-