scovich commented on code in PR #9454:
URL: https://github.com/apache/arrow-rs/pull/9454#discussion_r2874448790
##########
arrow-array/src/cast.rs:
##########
@@ -138,21 +138,24 @@ macro_rules! downcast_integer {
/// [`DataType`]: arrow_schema::DataType
#[macro_export]
macro_rules! downcast_integer_array {
- ($values:ident => $e:expr, $($p:pat $(if $pred:expr)* => $fallback:expr
$(,)*)*) => {
- $crate::downcast_integer_array!($values => {$e} $($p $(if $pred)* =>
$fallback)*)
- };
- (($($values:ident),+) => $e:expr, $($p:pat $(if $pred:expr)* =>
$fallback:expr $(,)*)*) => {
- $crate::downcast_integer_array!($($values),+ => {$e} $($p $(if $pred)*
=> $fallback)*)
- };
- ($($values:ident),+ => $e:block $($p:pat $(if $pred:expr)* =>
$fallback:expr $(,)*)*) => {
- $crate::downcast_integer_array!(($($values),+) => $e $($p $(if $pred)*
=> $fallback)*)
- };
- (($($values:ident),+) => $e:block $($p:pat $(if $pred:expr)* =>
$fallback:expr $(,)*)*) => {
+ ($($values:ident),+ => $e:block $($p:pat $(if $pred:expr)? =>
$fallback:expr $(,)?)*) => {
Review Comment:
given this observation:
> avoid wrapping in parentheses just to remove them later on. Note: that's
debatable as the base case now shows the inelegant unwrapped `$values` list.
Should we consider adjusting the match arms to favor tuple syntax, and also
fully support the values list form?
```rust
// canonical form: parenthetical arg(s) and block
(($($values:ident),+) => $e:block ...
// Turn $e into a block
(($($values:ident),+) => $e:expr ...
// Wrap $values in parentheses
($($values:ident),+ => $e:block ...
// Turn $e into a block _and_ wrap $values in parentheses
($($values:ident),+ => $e:expr ...
```
PRO: It's an implementation detail that the canonical form has to remove the
tuple-form arg parens before forwading to the `downcast_integer!` macro?
CON: Most use sites would pass a single arg, so requiring tuple form in the
canonical case would require a recursive invocation to turn `value` into
`(value)`. But only one recursive step instead of up to three like upstream has
been doing.
--
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]
