-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[stdlib] Optional.value should return a Reference #2179
Comments
I will take this task up. Thanks |
In this what should be passed into the lifetime component and the mutability component of the Reference? |
@lsh I was working on this |
@StandinKP didn't mean to step on any toes. This is a feature I wanted for a project, and I knew roughly how to wire up all the lifetimes and mutability in |
Nah man don't worry. Don't close it. I'll review the PR and let you know if any comments. Next time it would be good to ask so we won't be repeating efforts and we can grow this faster🤟🏻🤩 |
Change `Optional.value()` to return a `Reference` to `T` rather than `T`. Fixes modularml#2179. Signed-off-by: Lukas Hermann <lukashermann28@gmail.com>
Change `Optional.value()` to return a `Reference` to `T` rather than `T`. Fixes modularml#2179. Signed-off-by: Lukas Hermann <lukashermann28@gmail.com>
…) (#38062) Change `Optional.value()` to return a `Reference` to `T` rather than `T`. Fixes #2179. Signed-off-by: Lukas Hermann <lukashermann28@gmail.com> mojo-orig-commit: a904b34 --------- Co-authored-by: Lukas Hermann <lukashermann28@gmail.com> Co-authored-by: Joe Loser <joe@modular.com> MODULAR_ORIG_COMMIT_REV_ID: c07cde56dac507b04d8e9c1de83c82f63f3cae90
Review Mojo's priorities
What is your request?
The Optional docs for
value()
state that it should return aReference
when that is possible. Since the underlyingVariant
already returns a reference which is then dereferenced, this should be possible now.What is your motivation for this change?
This is a planned feature per the comment on the method.
Any other details?
No response
The text was updated successfully, but these errors were encountered: