eitsupi commented on code in PR #3197:
URL: https://github.com/apache/arrow-adbc/pull/3197#discussion_r2244160285
##########
rust/driver_manager/src/lib.rs:
##########
@@ -720,6 +712,7 @@ fn set_option_database(
ERR_ONLY_STRING_OPT,
Status::NotImplemented,
))?,
+ (_, _) => unreachable!(),
Review Comment:
Sorry for the lack of explanation.
Rust's pattern matching will cause a compilation error in the following
cases if enum is extended upstream.
```rs
match version {
AdbcVersion::V100 => ...,
AdbcVersion::V110 => ...,
}
```
~~This is the same regardless of whether the `non_exhaustive` attribute is
present or not,~~
but if the enum has the `non_exhaustive` attribute, ~~clippy will seem
generate an error in the code above and~~ request the addition of a wildcard
condition.
```rs
match version {
AdbcVersion::V100 => ...,
AdbcVersion::V110 => ...,
_ => ...,
}
```
If we write code like this, it will compile successfully even if the enum is
expanded when the dependent adbc_core is updated.
On the other hand, if there is no wildcard condition, an error will occur at
compile time, making it clear that we need to update to a newer version.
```rs
match version {
AdbcVersion::V100 => ...,
AdbcVersion::V110 => ...,
AdbcVersion::V200 => ...,
}
```
I think that Rust developers are usually familiar with pattern matching, so
using the `non_exhaustive` attribute here would only delay the discovery of the
problem (even if there is no error at compile time, there will eventually be an
error at runtime), so I think it would be better not to use it.
Edit: Sorry, I had some misunderstandings, so I've corrected the comment.
It seems that `non_exhaustive_omitted_patterns` needs to be used downstream
to avoid missing new pattern.
https://internals.rust-lang.org/t/stabilizing-non-exhaustive-omitted-patterns-or-a-better-alternative/21679
--
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]
