-
Notifications
You must be signed in to change notification settings - Fork 15
Exception Handling #64
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
Conversation
ext/symengine/ruby_basic.c
Outdated
| return result; | ||
| } else { | ||
| basic_free_stack(cbasic_operand2); | ||
| rb_raise(rb_eRuntimeError, "Runtime Error"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this error signify? The error message should be more descriptive than "Runtime Error", which does not give much information about what's actually going wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@v0dro this is in place until a exceptions are made in the SymEngine C++ code (right now it's all std::runtime_errors). Anyway I have written the code to be integrated into the cwrappers to catch multiple exceptions (for a method like parse this is needed). Then depending on the error code sent, the suitable error message will be displayed. This is still WIP. Please check symengine/symengine#1044 for the discussion on this :)
ext/symengine/symengine_utils.c
Outdated
| } else { | ||
| basic_free_stack(cbasic_operand1); | ||
| basic_free_stack(cbasic_operand2); | ||
| rb_raise(rb_eRuntimeError, "Runtime Error"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use raise_exception here.
45c9334 to
62351d1
Compare
ext/symengine/symengine_utils.c
Outdated
| basic_free_stack(cbasic_operand2); | ||
| int error_code = cwfunc_ptr(cresult, cbasic_operand1, cbasic_operand2); | ||
|
|
||
| if (error_code == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of 0, check for SYMENGINE_NO_EXCEPTION
|
Can you add some tests in ruby? |
ext/symengine/ruby_basic.c
Outdated
| cresult); | ||
| basic_free_stack(cbasic_operand2); | ||
|
|
||
| int error_code = cwfunc_ptr(cresult, this, cbasic_operand2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to use the enum for error codes here.
| result = Data_Wrap_Struct(Klass_of_Basic(cresult), NULL, cbasic_free_heap, | ||
| cresult); | ||
|
|
||
| symengine_exceptions_t error_code = cwfunc_ptr(cresult, this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use one convention. int or symengine_exceptions_t or CWRAPPER_OUTPUT_TYPE. All 3 are used in this PR.
|
Thanks for the PR. |
An example with
cbasic_divmethod.