Skip to content

Conversation

@dearblue
Copy link
Contributor

Previously it returned undef.
So for example, if there was a syntax error, the following code would cause SIGSEGV.

const char *code = "syntax ? error ?";
mrb_p(mrb, mrb_load_string(mrb, code));

This affects the behavior of the MRB_API function below:

  • mrb_load_file()
  • mrb_load_file_cxt()
  • mrb_load_detect_file_cxt()
  • mrb_load_string()
  • mrb_load_nstring()
  • mrb_load_string_cxt()
  • mrb_load_nstring_cxt()

I mentioned earlier why I want to change from undef to nil, but here's why I decided to change it:

  • Returns nil if an error occurred when returning from mrb_top_run().
  • The specific error can be found by confirming that mrb->exc is non-NULL.
  • Even if mrbc_context::no_exec is true, it is sufficient to check if it is either proc or nil.

However, this change can cause compatibility issues if it has already used mrb_undef_p() to distinguish between syntax errors.
This is the reason for making changes to the mrdb.c file (although the behavior does not change with or without changes).


Just to be sure.
I think the current behavior of returning undef is non-bug.

Previously it returned `undef`.
So for example, if there was a syntax error, the following code would cause `SIGSEGV`.

```c
const char *code = "syntax ? error ?";
mrb_p(mrb, mrb_load_string(mrb, code));
```

This affects the behavior of the `MRB_API` function below:
- `mrb_load_file()`
- `mrb_load_file_cxt()`
- `mrb_load_detect_file_cxt()`
- `mrb_load_string()`
- `mrb_load_nstring()`
- `mrb_load_string_cxt()`
- `mrb_load_nstring_cxt()`

I mentioned earlier why I want to change from `undef` to `nil`, but here's why I decided to change it:
- Returns `nil` if an error occurred when returning from `mrb_top_run()`.
- The specific error can be found by confirming that `mrb->exc` is non-`NULL`.
- Even if `mrbc_context::no_exec` is true, it is sufficient to check if it is either `proc` or `nil`.

However, this change can cause compatibility issues if it has already used `mrb_undef_p()` to distinguish between syntax errors.
This is the reason for making changes to the `mrdb.c` file (although the behavior does not change with or without changes).
@dearblue dearblue requested a review from matz as a code owner June 26, 2021 02:29
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.

1 participant