Skip to content
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

update RFC-2895 #1

Closed
wants to merge 3 commits into from
Closed

update RFC-2895 #1

wants to merge 3 commits into from

Conversation

waynr
Copy link

@waynr waynr commented Jul 10, 2023

This PR does three things:

  • it removes a reference to the backtrace unstable feature (which I believe has been stabilized) and the defunct backtrace method on Error
  • it removes a big chunk of the old proof of concept code, references the current unstable implementation in the standard library, and adds some of the public-facing API snippets with explanation as to how they are intended to be used
  • adds a reference to RFC 3192 in the "prior art" section
    • considered "prior" because I am proposing that provide_any and error_generic_member_access be merged

One thing I think is still missing from my changes to this RFC is a bit more detail about how <dyn Error>.request_ref and <dyn Error>.request_value are implemented in terms of the new private type core::any::TaggedOption and module core::any::tags. I think that's appropriate because of the documentation on the reference-level explanation section: https://github.com/rust-lang/rfcs/blob/master/0000-template.md#reference-level-explanation

Its interaction with other features is clear.
It is reasonably clear how the feature would be implemented.

@yaahc I think your offer to sync up and discuss this further would help me flesh this section out, unless you don't think it's necessary to go into any further detail.

@waynr waynr marked this pull request as draft July 10, 2023 22:13
@yaahc
Copy link
Owner

yaahc commented Jul 19, 2023

@yaahc I think your offer to sync up and discuss this further would help me flesh this section out, unless you don't think it's necessary to go into any further detail.

I'd be happy to help flesh it out, though I don't have strong opinions on whether it is necessary. It would probably be fine to push the rest of the changes, point out the gap, and ask the libs team what they think rather than adding it proactively, so I guess my preference would be to wait and see, but if you'd prefer to expand on it I'm happy to support that work.

@waynr
Copy link
Author

waynr commented Jul 19, 2023

my preference would be to wait and see, but if you'd prefer to expand on it I'm happy to support that work.

That sounds good to me, save additional work for future me only if it's necessary :).

So this is a PR to update your PR -- is that the best way to proceed or should I just re-open RFC-2895 so I can work on and get feedback directly on the PR that will be merged into master? Would the RFC name change in that case?

@yaahc
Copy link
Owner

yaahc commented Jul 20, 2023

So this is a PR to update your PR -- is that the best way to proceed or should I just re-open RFC-2895 so I can work on and get feedback directly on the PR that will be merged into master? Would the RFC name change in that case?

That sounds like a smart plan. And if by name change you mean would the RFC number change? yeah

@waynr
Copy link
Author

waynr commented Jul 20, 2023

@yaahc here's the new PR targeted at master branch with my updates added: rust-lang#3461

@waynr waynr closed this Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants