core/any: remove Provider trait, rename Demand to Request - #113464
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This touches on two WIP features:
error_generic_member_accesserror_generic_member_access#99301provide_anyThe changes in this PR are intended to address libs meeting feedback summarized by @Amanieu in #96024 (comment)
The specific items this PR addresses so far are:
I went with
Requesthere since it sounds nicer, but I'm mildly concerned that at first glance it could be confused with the use of the word in networking context.The net impact this PR has is that examples which previously looked like
now look like
These are methods that based on the type hint when called return an
Option<T>of that type. I'll admit I don't fully understand how that's done, but it involvescore::any::tags::Typeandcore::any::TaggedOption, neither of which are exposed in the public API, to construct aRequestwhich is then passed to theError.providemethod.Something that I'm curious about is whether or not they are essential to the use of
Requesttypes (prior to this PR referred to asDemand) and if so does the fact that they are kept private imply thatRequests are only meant to be constructed privately within the standard library? That's what it looks like to me.These methods ultimately call into code that looks like:
As far as the
RequestAPI is concerned, one suggestion I would like to make is that the previous example should look more like this:This makes it possible for anyone to construct a
Requestfor use in their own projects without exposing an implementation detail likeTaggedOptionin the API surface.Otherwise noteworthy is that I had to add
pub(crate)on bothcore::any::TaggedOptionandcore::any::tagssinceRequests now need to be constructed in thecore::errormodule. I considered movingTaggedOptioninto thecore::errormodule but again I figured it's an implementation detail ofRequestand belongs closer to that.At the time I am opening this PR, I have not yet looked into the following bit of feedback:
This is what I'll focus on next while waiting for feedback on the progress so far. I suspect that learning more about the type IDs will help me understand the need for
TaggedOptiona little better.