Implement `mrb_protect`. #2845

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
6 participants
@take-cheeze
Contributor

take-cheeze commented Jun 21, 2015

This pull request implements mrb_protect, mrb_ensure, mrb_rescue, mrb_rescue_exceptions.
(mrb_rescue_exceptions is implementation of rb_rescue2.)
The 4 APIs are placed to "mruby-error" mrbgem which is included to 'default' gembox for now.

The changes to Build#compile_as_cxx is for missing directory when generating C++ file in some cases.

take-cheeze added some commits Jun 21, 2015

Implement `mrb_protect`, `mrb_ensure`, `mrb_rescue`, `mrb_rescue_exce…
…ptions`.

(`mrb_rescue_exceptions` is mruby implementation of `rb_rescue2`.)
Closes #2844, closes #2837.
@mattn

This comment has been minimized.

Show comment
Hide comment
Contributor

mattn commented Jun 21, 2015

@cremno

This comment has been minimized.

Show comment
Hide comment
@cremno

cremno Jun 21, 2015

Contributor

Great! But I see one issue. Your test code even contains it:

return mrb_rescue_exceptions(mrb, protect_cb, b, protect_cb, r, E_TYPE_ERROR, NULL);

NULL can expand to 0 (int), so one always has to write at least (struct RClass*)0. Why not change the function to take a pointer to an array and its length instead? Then it'd be type-safe and mruby can't and doesn't have to be compatible to CRuby anyway. However there's also a new downside: an extra variable or, if a C99 compiler is used, a compound literal is needed . There's also the mrb_funcall() compromise (length + varargs).

Contributor

cremno commented Jun 21, 2015

Great! But I see one issue. Your test code even contains it:

return mrb_rescue_exceptions(mrb, protect_cb, b, protect_cb, r, E_TYPE_ERROR, NULL);

NULL can expand to 0 (int), so one always has to write at least (struct RClass*)0. Why not change the function to take a pointer to an array and its length instead? Then it'd be type-safe and mruby can't and doesn't have to be compatible to CRuby anyway. However there's also a new downside: an extra variable or, if a C99 compiler is used, a compound literal is needed . There's also the mrb_funcall() compromise (length + varargs).

@jbreeden

This comment has been minimized.

Show comment
Hide comment
@jbreeden

jbreeden Jun 21, 2015

Contributor

Hurray! Regarding @cremno's comment, +1 for the array argument idea. It could still be NULL terminated, without sacrificing type safety (or go the length + array route... either way).

Thanks, @take-cheeze!

Contributor

jbreeden commented Jun 21, 2015

Hurray! Regarding @cremno's comment, +1 for the array argument idea. It could still be NULL terminated, without sacrificing type safety (or go the length + array route... either way).

Thanks, @take-cheeze!

@take-cheeze

This comment has been minimized.

Show comment
Hide comment
@take-cheeze

take-cheeze Jun 22, 2015

Contributor

@cremno
Array of exception classes seems nice.
The variadic API is confusing and I've changed the API name too.
I'm thinking of following API:

MRB_API mrb_value mrb_rescue_exceptions(
    mrb_state *mrb, mrb_func_t body, mrb_value b_data,
    mrb_func_t rescue, mrb_value r_data,
    mrb_int classes_len, struct RClasss **classes);
Contributor

take-cheeze commented Jun 22, 2015

@cremno
Array of exception classes seems nice.
The variadic API is confusing and I've changed the API name too.
I'm thinking of following API:

MRB_API mrb_value mrb_rescue_exceptions(
    mrb_state *mrb, mrb_func_t body, mrb_value b_data,
    mrb_func_t rescue, mrb_value r_data,
    mrb_int classes_len, struct RClasss **classes);
@take-cheeze

This comment has been minimized.

Show comment
Hide comment
@take-cheeze

take-cheeze Jun 22, 2015

Contributor

Done changing to array of classes API. ( 15c869f )

Contributor

take-cheeze commented Jun 22, 2015

Done changing to array of classes API. ( 15c869f )

@Asmod4n

This comment has been minimized.

Show comment
Hide comment
@Asmod4n

Asmod4n Jul 1, 2015

Contributor

yay

Contributor

Asmod4n commented Jul 1, 2015

yay

@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz Jul 15, 2015

Member

merged

Member

matz commented Jul 15, 2015

merged

@matz matz closed this Jul 15, 2015

@take-cheeze take-cheeze deleted the take-cheeze:mrb_protect branch Jun 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment