Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mrb_get_args_a() #5596

Merged
merged 3 commits into from Aug 24, 2022
Merged

Add mrb_get_args_a() #5596

merged 3 commits into from Aug 24, 2022

Conversation

sasq64
Copy link
Contributor

@sasq64 sasq64 commented Dec 11, 2021

This pull request adds a new function, mrb_get_args_a(), which works exactly like mrb_get_args() except that it takes
the pointer arguments as an array instead of varargs.

I needed to add this for my mruby binder library that I want to release publicly. It allows you to write code like:

(from my test cases)

    auto* ruby = mrb_open();

    auto* person_class = mrb::make_class<Person>(ruby, "Person");
    mrb::add_method<Person>(
        ruby, "age=", [](Person* p, int age) { p->age = age; });
    mrb::add_method<Person>(
        ruby, "age", [](Person const* p) { return p->age; });

    mrb::add_method<Person>(ruby, "copy_from", [](Person* p, Person* src) {
        p->name = src->name;
        p->age = src->age;
    });

    mrb::add_method<Person>(
        ruby, "dup", [](Person const* p) { return new Person(*p); });

    auto other_age = mrb_load_string(ruby,
        "person = Person.new ; person.age = 5 ; other = "
        "Person.new ; other.copy_from(person) ; person.age = 2 ; other.age");
    CHECK(mrb::value_to<int>(other_age) == 5);

@sasq64 sasq64 requested a review from matz as a code owner December 11, 2021 13:31
include/mruby/gc.h Show resolved Hide resolved
src/class.c Outdated Show resolved Hide resolved
Made mrb_get_args_v() static
@sasq64
Copy link
Contributor Author

sasq64 commented Dec 12, 2021

Good points. Updated my branch.

@sasq64
Copy link
Contributor Author

sasq64 commented Dec 19, 2021

@dearblue Anything else I need to do ?

@dearblue
Copy link
Contributor

I'm a third party just like you, so I don't have the authority to commit.
Please wait for @matz's decision.

Comment on lines +933 to +934
MRB_API mrb_int
mrb_get_args_v(mrb_state *mrb, mrb_args_format format, void** ptr, va_list ap)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having an MRB_API (public API) with a va_list parameter will make it difficult to provide an API and ABI compatible MRB API implementation outside of C. Is it intentional that this function is marked as part of the public API?

@matz
Copy link
Member

matz commented Jul 24, 2022

Functions with va_args should be specific to C API (by nature). You can safely ignore it.
And is mrb_get_args_a() above fit your need to provide Rust API?

@lopopolo
Copy link
Contributor

And is mrb_get_args_a() above fit your need to provide Rust API?

yes it does. thank you!

@matz
Copy link
Member

matz commented Jul 24, 2022

OK, I will merge this PR (with some updates).

matz added a commit that referenced this pull request Aug 24, 2022
@matz matz merged commit eaa89fb into mruby:master Aug 24, 2022
dearblue added a commit to dearblue/mruby that referenced this pull request Feb 4, 2024
For reference, the flexible array was introduced by commit 3ab2f93 (mruby#2997).
Subsequently changed for compatibility with C++ by commit 2493972 (mruby#5596).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants