plugin: type-check errors always report "parameter number 1"

Open
#960 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
Half a day
Newbie friendliness
70/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
c, cmake, cpp

Research direction

Start with the EXTENSION_FUNCTION_CHECK_ITERATOR macro in source/plugin/include/plugin/plugin_interface.hpp (around line 76), where the index is stringified instead of printed. Then check the preprocessed load() in the cli_core_plugin source and the metacall_cli_core_plugin_test CMakeLists.txt, whose early return depends on the order of add_subdirectory calls in source/CMakeLists.txt. Done when the message names the wrong parameter's index, a test checks the second-parameter message, and the test actually runs under ctest.

Written by the indexing model from the issue text.

Description

bug

🐛 Bug Report

When a plugin function gets an argument of the wrong type, the error message always says "The parameter number 1", whichever parameter was wrong.

Expected Behavior

The message names the parameter that has the wrong type, for example "The parameter number 2" when the second argument is wrong.

Current Behavior

Every type error says "The parameter number 1":

args[0] = 5 (Int)      -> Failed to load a script. The parameter number 1 requires a value of type String, received: Int
args[1] = "x" (String) -> Failed to load a script. The parameter number 1 requires a value of type Array, received: String

EXTENSION_FUNCTION_CHECK_ITERATOR builds the message with this (plugin_interface.hpp#L76):

PREPROCESSOR_STRINGIFY(PREPROCESSOR_ARGS_COUNT(iterator))

iterator is the index that PREPROCESSOR_FOR passes in (0, 1, ...). PREPROCESSOR_ARGS_COUNT(iterator) counts how many macro arguments that is, which is always 1, instead of using its value. The preprocessed load() in cli_core_plugin.cpp shows it: both checks have the string "1".

Possible Solution

Print the index instead of counting it, starting from 1 like extension_function_check_array in cli_core_plugin.cpp already does ("#" << (i + 1)):

ss << error ". The parameter number " << ((iterator) + 1) << " requires a value of type " << ...

And add a test in metacall_cli_core_plugin_test that checks the message for the second parameter. I'd also fix the typo "argumens" in the argument count message of the same file, if that's OK.

Related problem I found while adding the test: metacall_cli_core_plugin_test is never built. Its CMakeLists.txt returns early with NOT TARGET cli_core_plugin, but source/CMakeLists.txt adds tests before cli, so that target doesn't exist yet when the check runs. Three more tests have the same check and are skipped too: metacall_cli_core_plugin_await_test, metacall_plugin_extension_destroy_order_test and metacall_plugin_extension_invalid_path_test (since 3a631c67 and 24db326a in 2024). None of them appear in the Linux CI logs (for example, the GCC debug job of the last develop run, 212 tests).

Moving add_subdirectory(cli) before add_subdirectory(tests) fixes it. Locally the skipped tests pass after that (the await test needs the Node port, which I didn't build), and the full ctest passes 111/111. Without it, the new test wouldn't run in CI. Should I include it in the same PR as a separate commit, or open a separate issue for it?

Steps to Reproduce
  1. Build with the CLI (OPTION_BUILD_LOADERS_NODE=ON, which builds cli_core_plugin).
  2. Load the plugin and call load with a String as the second argument (it expects an Array). I used a String because MetaCall converts numbers to an Array before the call, so they never reach this check.
void *handle = NULL;
metacall_load_from_configuration(CLI_CORE_PLUGIN_PATH, &handle, allocator);

void *args[2] = { metacall_value_create_string("node", 4), metacall_value_create_string("x", 1) };
void *ret = metacallhv_s(handle, "load", args, 2);

struct metacall_exception_type ex;
metacall_error_from_value(ret, &ex);
printf("%s\n", ex.message); /* "... The parameter number 1 requires a value of type Array, received: String" */
  1. The message says parameter 1, but the wrong one is the second.
Context (Environment)

Found while working on #560. develop at 11e2b835, Ubuntu 24.04, GCC 13.3. It's in the macro, so every plugin that uses EXTENSION_FUNCTION_CHECK is affected (CLI core and sandbox plugins, sandbox plugin).

I already have the fix, the test and the CMake change working locally (the full ctest passes with GCC and Clang), and I'd like to open the PR for this myself. It touches the same header as #954, but different lines, and it applies cleanly on top of it. Can I go ahead?

Dominant language
C
Stars
1.8k
Forks
266
Avg merge
2d 16h
Merged PRs (30d)
14

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from metacall/core

All issues in metacall/core

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.