yallop / ocaml-ctypes

Library for binding to C libraries using pure OCaml
MIT License
363 stars 95 forks source link

Fix -Wunused-argument for generated C stubs for bytecode #729

Closed dannywillems closed 11 months ago

dannywillems commented 1 year ago

This is a draft, untested. Related to https://github.com/yallop/ocaml-ctypes/issues/248#issuecomment-1440045648

dannywillems commented 1 year ago

Running find . -name "*stubs*.c" | xargs grep "argc" after make && make test gives some generated stubs. Picking randomly ./tests/test-returning-errno-lwt-jobs/generated_stubs.c and checking it is fine:

Before:

value cstubs_tests_2_sixargs_byte6 (value *argv, int argc)
{
  return
    cstubs_tests_2_sixargs(argv[0], argv[1], argv[2], argv[3], argv[4],
                           argv[5]);
}
value cstubs_tests_2_sixargs_byte6 (value *argv, int argc)
{
  (void)(argc);
return
                  cstubs_tests_2_sixargs(argv[0], argv[1], argv[2], argv[3],
                                         argv[4], argv[5]);
}

Do we want a better formatting?

Also, I don't see any make rules to compile the C code to check we can compile. Can you help me? IMO, the MR is ready. Changing the title.

yallop commented 1 year ago

Do we want a better formatting?

Yes; it looks better after 66d1bfd:

value cstubs_tests_2_sixargs_byte6 (value *argv, int argc)
{
  (void)(argc);
  return
    cstubs_tests_2_sixargs(argv[0], argv[1], argv[2], argv[3], argv[4],
                           argv[5]);
}

You may need to rebuild from clean to see it. I'm not sure the test build rules capture the dependencies precisely enough to pick up the change in the cstubs library.

Also, I don't see any make rules to compile the C code to check we can compile. Can you help me?

The rules are in Makefile.rules (e.g.) and Makefile.tests. Running make test is sufficient to build the generated C code. The object file for tests/test-returning-errno-lwt-jobs/generated_stubs.c should be in _build/tests/test-returning-errno-lwt-jobs/generated_stubs.o.

yallop commented 11 months ago

Thank you!