-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Tracking Issue for Result<Option> and Option<Result> Conversion #47338
Comments
A nice-to-have but not required would be someway for us to make this general so it can be used with other types, for example with I saw |
Does it make sense to implement |
Then perhaps |
Nice addition 👍 The generalization of this, which we should experiment with when we get GATs, should just be |
Conversions for |
I don't think so. The code below doesn't compile. #![feature(catch_expr)]
fn transpose<T, E1, E2>(x: Result<Result<T, E1>, E2>) -> Result<Result<T, E2>, E1> {
do catch {
Ok(x??)
}
} |
@qnighy that's because you're missing |
@clarcharr perhaps are we talking about different functions? My understanding is that |
Also, is it just me, or does |
Hi everyone, is there anything which prevents this useful feature from being stabilized? I've come here from a link from the docs about the Thanks :) |
Ping @rust-lang/libs this has been sitting around for a year now and I don't expect we'll see much more experimentation with it if folks have to rely on a nightly feature. What would y'all think about stabilizing? |
Sounds ok to me. @rfcbot fcp merge This issue tracks these methods: impl<T, E> Option<Result<T, E>> {
pub fn transpose(self) -> Result<Option<T>, E> {…}
}
impl<T, E> Result<Option<T>, E> {
pub fn transpose(self) -> Option<Result<T, E>> {…}
} |
Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
This will be an improvement under any name, but here's an argument for stabilizing this as
The first point feels particularly compelling. This method will be used in all kinds of code. People reviewing code dealing with matrices will strongly expect |
The term |
Perhaps going with |
Agreed those would be better than The semantic argument for Perhaps we should consider the merits of |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
As someone who's already well aware of these two functions, I would not mind the name However, it has one disadvantage that should be considered: people will likely have a hard time searching for functions named Maybe it might be better to pick a slightly ambiguous name like |
RE My main concern is discoverability. Once you get past the "how will people think to see if this exists" (maybe a help in a compiler error?), there is the "how will it stand out". Personally, I gloss over weird names in libraries (e.g. |
For discoverability, could the documentation for the |
Regarding discoverability, it seems unlikely that anyone will first find these methods by name regardless of what we call them. There's just not a common name for this operation. People will find these 1) by searching the web for "convert Rust Result Option to Option Result" or "chain Rust Result methods with Option" or other variations, 2) by scanning the Option or Result documentation for a method of the correct signature, or 3) by being introduced to the concept through the book, through blogs, or through seeing discussions such as this one. Regarding matrices, as soon as you have a function that returns |
The final comment period, with a disposition to merge, as per the review above, is now complete. |
Stabilize `transpose_result` in 1.33 fixes #47338. FCP completed: #47338 (comment) r? @alexreg
Someone proposed to have something close to Hoogle for search this kind of feature. |
Initial implementation in #47193.
The text was updated successfully, but these errors were encountered: