kevingurney opened a new pull request, #36273:
URL: https://github.com/apache/arrow/pull/36273

   ### Rationale for this change
   
   1. To make working with `arrow::Result<T>` values easier in the C++ code for 
the MATLAB interface, it would be useful to have a `MATLAB_ASSIGN_OR_ERROR` 
macro that mirrors the design of the 
[`ARROW_ASSIGN_OR_RAISE`](https://github.com/apache/arrow/blob/320ecbd119f26cb2f8d604ed84aae2559dbc0e26/cpp/src/arrow/result.h#L475)
 macro.
   2. @kou helpfully suggested this here: 
https://github.com/apache/arrow/pull/36190#discussion_r1237932111.
   3. There is already a 
[`MATLAB_ERROR_IF_NOT_OK`](https://github.com/apache/arrow/blob/e3eb5898e75a0b901724f771a7e2de069993a33c/matlab/src/cpp/arrow/matlab/error/error.h#L26)
 macro, so this would roughly follow the approach of that macro.
   
   ### What changes are included in this PR?
   
   1. Added a new macro `MATLAB_ASSIGN_OR_ERROR(lhs, rexpr, id)`.
   2. Added additional comments describing the error macros.
   3. Updated call sites (i.e. `numeric_array.h` and `boolean_array.cc`) where 
`arrow::Result<T>` is being returned to use new `MATLAB_ASSIGN_OR_ERROR` macro 
where possible.
   
   **Example**
   
   ```matlab
   MATLAB_ASSIGN_OR_ERROR(auto array, array_builder.Finish(), 
"arrow:matlab:array:builder:FailedToBuildArray");
   ``` 
   
   ### Are these changes tested?
   
   Yes.
   
   1. The client code that was changed to use `MATLAB_ASSIGN_OR_ERROR` compiles 
and works as expected.
   2. **Note**: We intentionally left the other macros 
`MATLAB_ERROR_IF_NOT_OK_WITH_CONTEXT` and `MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT` 
commented out for the time being, since we don't have any client code to 
exercise these macros. We can uncomment these when we have client code that 
would benefit from using these macros. For reference, non-static `Proxy` member 
functions return an error ID to MATLAB by assigning to the `context.error` 
property of the input `libmexclass::proxy::method::Context` object (see [this 
example](https://github.com/mathworks/libmexclass/blob/77f3d72c22a9ddab7b54ba325d757c3e82e57987/example/proxy/Car.cpp#L46)).
 This is different than how errors are returned  to MATLAB from within a 
`Proxy` static `make()` method.
   
   ### Are there any user-facing changes?
   
   Yes.
   
   1. There is now a new `MATLAB_ASSIGN_OR_ERROR` macro that can be used for 
simplifying the process of extracting a value of type `T` from an expression 
that returns an `arrow::Result<T>` or returning an appropriate error to MATLAB 
if the status of the `Result` is not "OK".
   
   ### Future Directions
   
   1. Consider modifying `mathworks/libmexclass` so that non-static `Proxy` 
member functions return a `Result`-like object, instead of requiring clients to 
assign to `context.error`. This might help avoid the need for two different 
"kinds" of macros - i.e. `MATLAB_ASSIGN_OR_ERROR` / 
`MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT` and `MATLAB_ERROR_IF_NOT_OK` / 
`MATLAB_ERROR_IF_NOT_OK_WITH_CONTEXT`.
   2. Uncomment additional error macros when we have valid client use cases.
   
   ### Notes
   
   1. Thank you @sgilmore10 for your help with this pull request!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to