Stream: project-error-handling

Topic: generic member access


view this post on Zulip Nick Cameron (Aug 04 2021 at 00:38):

I've been experimenting with the generic member access implementation in dyno. I think I have a similar but simpler API for the 'object provider' case. I think it handles the same requirements - no explicit type id comparison, supports references (at least those with a lifetime bound by self) and ?Sized types, does not require Box, supports by value.

view this post on Zulip Nick Cameron (Aug 04 2021 at 00:38):

https://gist.github.com/nrc/293e88c0e796114ef181d1b2d5e1ac7e

view this post on Zulip Nick Cameron (Aug 04 2021 at 00:40):

One worry is casting too and from *mut u8, which I don't recall if that is sound or not. The other worry is that maintaining the lifetime of references is pretty fragile and I'm not convinced it's sound in all variance scenarios.

view this post on Zulip Nick Cameron (Aug 04 2021 at 00:40):

Plus its probably wildly broken in ways I don't understand

view this post on Zulip Nick Cameron (Aug 04 2021 at 00:41):

Anyway, please take a look and tell me what I screwed up!

view this post on Zulip Nick Cameron (Aug 04 2021 at 01:50):

I’m not sure how it handles the closure use case for async runtimes

view this post on Zulip Nick Cameron (Aug 04 2021 at 02:04):

I found one lifetime error already!

view this post on Zulip Nick Cameron (Aug 04 2021 at 02:59):

Gist updated to address the lifetime issue and add some comments

view this post on Zulip Nick Cameron (Aug 04 2021 at 03:42):

In terms of pros/cons, with dyno, the Tags are more first class, which means you can write your own for your types to support more complicated types such as &'a Foo<'b> (my provider only supports &'a Foo<'static>), however, the end user does have to write these tags and if you have to specify the generic type parameters, you have to specify the tag rather than the type. C.f., my proposal where the type is specified. E.g., err.context::<TagType>()

view this post on Zulip Jane Lusby (Aug 04 2021 at 03:43):

Is your version of the provider API implemented ontop of Tag?

view this post on Zulip Jane Lusby (Aug 04 2021 at 03:43):

Haven't had a chance yet to look at it in detail

view this post on Zulip Jane Lusby (Aug 04 2021 at 03:44):

Looks like no

view this post on Zulip Nick Cameron (Aug 04 2021 at 03:59):

No, it uses type tags, but they are not abstracted with a trait

view this post on Zulip Jane Lusby (Aug 05 2021 at 18:32):

taking a look at the version you setup @Nick Cameron it doesn't seem like it would still cover the futures executor case

view this post on Zulip Jane Lusby (Aug 05 2021 at 18:33):

or at least i don't understand how it would if T: 'static

view this post on Zulip Jane Lusby (Aug 05 2021 at 18:34):

ah i guess that's what you meant by "for the object provider case"

view this post on Zulip Jane Lusby (Aug 05 2021 at 18:34):

seems like a reasonable option to include in the RFC

view this post on Zulip Nick Cameron (Aug 11 2021 at 04:06):

Thanks for looking it over! I've been on vacation for a few days, so sorry for the lack of response.

view this post on Zulip Nick Cameron (Aug 11 2021 at 04:07):

I talked to Nika last week and understand dyno a bit better now, in particular how it can support types with lifetimes by having the user write the Tag impls

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:10):

Nice

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:10):

Did she end up pushing some new versions of Dyno?

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:10):

I think she did, but it may be on a branch

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:10):

One sec

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:11):

https://github.com/mystor/dyno/tree/min_magic

view this post on Zulip Nick Cameron (Aug 11 2021 at 04:17):

oh cool, I hadn't seen that

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:21):

Screenshot_20210810-212017~2.png

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:22):

I also realized that last time I checked bevy had an identical crate

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:22):

Trait*

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:22):

That it uses for its own type erasure logic

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:22):

I'm not sure if there's any significant benefit to having a common trait but it's definitely useful prior art

view this post on Zulip Nick Cameron (Aug 11 2021 at 04:24):

:+1:

view this post on Zulip Jane Lusby (Aug 11 2021 at 04:24):

https://docs.rs/bevy/0.5.0/bevy/ecs/system/trait.SystemParam.html

view this post on Zulip Jane Lusby (Aug 11 2021 at 06:02):

Actually it might be https://docs.rs/bevy/0.5.0/bevy/ecs/system/trait.SystemParamFetch.html

view this post on Zulip Jane Lusby (Aug 11 2021 at 06:02):

And even then it's not the same

view this post on Zulip Nick Cameron (Aug 18 2021 at 04:01):

Using the new dyno, I was able to get the best of both worlds: by adding specialised methods like provide_value and context_value, you can do the generic member access thing and in the common case never know anything about the whole Tag setup, but if you need to support complex cases (like closures for the async use case) then you can still define your own tags and use a generic method with an explicit tag

view this post on Zulip Jane Lusby (Aug 18 2021 at 17:25):

:heart:

view this post on Zulip Nick Cameron (Aug 23 2021 at 03:48):

Current version of the provider API I plan to propose is at https://github.com/nrc/provide-any

view this post on Zulip Nick Cameron (Aug 23 2021 at 03:50):

It is based on Nika's dyno min-max branch, but modified a bit, in particular so that Error's API for common types is a bit more ergonomic, and to only support generic member access (downcasting based on type tags is kept as an internal detail, but could be exposed later)

view this post on Zulip Nick Cameron (Aug 23 2021 at 03:50):

I'm writing the RFC today

view this post on Zulip Nick Cameron (Aug 23 2021 at 03:53):

Any feedback on the API would be appreciated (I realise the code is a bit scruffy, will polish as I go along)

view this post on Zulip Nick Cameron (Aug 23 2021 at 03:53):

I expect to have an RFC draft this week

view this post on Zulip Nick Cameron (Aug 23 2021 at 03:53):

ready for review

view this post on Zulip Jane Lusby (Aug 23 2021 at 04:03):

Awesome

view this post on Zulip Jane Lusby (Aug 23 2021 at 04:03):

I'll take a look at it first thing in the morning

view this post on Zulip Nick Cameron (Aug 23 2021 at 04:25):

Thanks!

view this post on Zulip Nick Cameron (Aug 23 2021 at 05:01):

@Jane Lusby other than adding context in anyhow (and Eyre) are you aware of any error libraries which have functionality similar to generic member access?

view this post on Zulip Jane Lusby (Aug 23 2021 at 05:17):

There's traced error in the tracing error library that adds a span trace to an error that can be extracted through a dyn error via type erasure and downcast

view this post on Zulip Jane Lusby (Aug 23 2021 at 05:18):

Which is kind of the inspiration for the whole generic member access thing or the workaround for the lack thereof

view this post on Zulip Jane Lusby (Aug 23 2021 at 05:18):

It's kind of both

view this post on Zulip Nick Cameron (Aug 23 2021 at 05:24):

thanks!

view this post on Zulip Jane Lusby (Aug 23 2021 at 18:24):

oh that blanket impl of Provider for Error types is nice

view this post on Zulip Jane Lusby (Aug 23 2021 at 18:24):

does that just work? I figured we wouldn't be able to add a new super trait

view this post on Zulip Jane Lusby (Aug 23 2021 at 18:25):

I feel like that could easily cause coherence issues later on if people wanted to be able to use the Provider trait separately

view this post on Zulip Jane Lusby (Aug 23 2021 at 18:25):

I'm imagining future incompatibility errors and or overlap

view this post on Zulip Jane Lusby (Aug 23 2021 at 18:26):

i guess that's only an issue in the crate that defines Provider maybe

view this post on Zulip Jane Lusby (Aug 23 2021 at 18:26):

and downstream they will always know if they impl Provider or Error

view this post on Zulip Jane Lusby (Aug 23 2021 at 18:27):

Nick Cameron said:

It is based on Nika's dyno min-max branch, but modified a bit, in particular so that Error's API for common types is a bit more ergonomic, and to only support generic member access (downcasting based on type tags is kept as an internal detail, but could be exposed later)

Am I correct in understanding that the type tags being made an internal detail hasn't happened yet for the code you linked?

view this post on Zulip Jane Lusby (Aug 23 2021 at 18:27):

It seems to have a few pub APIs that expose the type tag interfaces atm

view this post on Zulip Jane Lusby (Aug 23 2021 at 18:29):

overall looks good, I will have some comments about the code but I'll wait until you've polished it and got the RFC written and are ready for review

view this post on Zulip Nick Cameron (Aug 23 2021 at 22:00):

I think that if we add the trait and the impl at the same time, then it is OK. There would only be a coherence issue if someone wanted to implement Provider for their own Error type, which they should never need to do

view this post on Zulip Nick Cameron (Aug 23 2021 at 22:01):

type tags are exposed, but Tagged and related things are not

view this post on Zulip Jane Lusby (Aug 23 2021 at 22:01):

Nick Cameron said:

I think that if we add the trait and the impl at the same time, then it is OK. There would only be a coherence issue if someone wanted to implement Provider for their own Error type, which they should never need to do

I'm not certain that's the only way this could cause issues

view this post on Zulip Nick Cameron (Aug 23 2021 at 22:01):

I think it is a nice compomise

view this post on Zulip Jane Lusby (Aug 23 2021 at 22:01):

imagine someone wants to have provider as a super trait the same way that Error does

view this post on Zulip Jane Lusby (Aug 23 2021 at 22:01):

and they add a blanket impl

view this post on Zulip Nick Cameron (Aug 23 2021 at 22:02):

super trait of what?

view this post on Zulip Jane Lusby (Aug 23 2021 at 22:02):

of their own local trait

view this post on Zulip Jane Lusby (Aug 23 2021 at 22:02):

something where they want to add a provide_context method to some trait same as the Error trait does

view this post on Zulip Jane Lusby (Aug 23 2021 at 22:02):

im trying to imagine the ways this can cause overlap

view this post on Zulip Jane Lusby (Aug 23 2021 at 22:02):

but I want to make sure we don't run into issues like with From and Error

view this post on Zulip Jane Lusby (Aug 23 2021 at 22:02):

seems like its probably fine

view this post on Zulip Jane Lusby (Aug 23 2021 at 22:03):

I mean, they wouldn't be able to add impls of their trait for foreign types

view this post on Zulip Nick Cameron (Aug 23 2021 at 22:03):

there's an Error bound on the blanket impl, so it should be fine

view this post on Zulip Nick Cameron (Aug 23 2021 at 22:03):

(they should have Provider and Error as super traits)

view this post on Zulip Nick Cameron (Aug 27 2021 at 01:51):

Draft RFC is ready for review: https://github.com/nrc/rfcs/pull/1 It is not finished, though, still a couple of large TODOs in the reference section, but I doubt I'll address them for a few days so thought I'd ask for feedback now

view this post on Zulip Nick Cameron (Aug 27 2021 at 01:51):

Anyone interested, please take a look!

view this post on Zulip Jane Lusby (Aug 27 2021 at 01:55):

oh shiit

view this post on Zulip Jane Lusby (Aug 27 2021 at 01:55):

taking a look now

view this post on Zulip Nick Cameron (Aug 27 2021 at 01:57):

thanks!

view this post on Zulip Jane Lusby (Aug 27 2021 at 02:08):

suddenly a refresh button

view this post on Zulip Jane Lusby (Aug 27 2021 at 02:08):

lol

view this post on Zulip Nick Cameron (Aug 27 2021 at 02:09):

OK, that was a lie, I addressed one of the reference section TODOs. I won't manage the other before next week though

view this post on Zulip Jane Lusby (Aug 27 2021 at 02:09):

what was a lie?

view this post on Zulip Jane Lusby (Aug 27 2021 at 02:09):

ooh

view this post on Zulip Jane Lusby (Aug 27 2021 at 02:09):

the thing about the todos

view this post on Zulip Nick Cameron (Aug 27 2021 at 02:09):

yeah

view this post on Zulip Jane Lusby (Aug 27 2021 at 02:09):

im almost done with the guide level seciton

view this post on Zulip Nick Cameron (Aug 27 2021 at 02:10):

hah, then my update was just in time!

view this post on Zulip Jane Lusby (Aug 27 2021 at 02:29):

posted my comments

view this post on Zulip Jane Lusby (Aug 27 2021 at 02:29):

looks great btw

view this post on Zulip Nick Cameron (Aug 27 2021 at 02:35):

thanks!

view this post on Zulip Gus Wynn (Aug 27 2021 at 06:10):

qq on https://github.com/nrc/provide-any, why is the TypeTag impl for Value T: 'static and not T: ?Sized + 'static? I know a !Sized + 'static object is pretty hard to come by (impossible?), but the code compiles fine with the looser bound

edit: ignore me, it doesnt compile

view this post on Zulip Sean Chen (he/him) (Aug 27 2021 at 17:09):

Wanted to also chime in and say that the RFC does a great job explaining the particulars of provide_any, which I was really struggling to wrap my head around when I was staring at the code. Great work @Nick Cameron :smile:

view this post on Zulip Jakub Duchniewicz (Aug 28 2021 at 11:52):

I also went through the RFC, finally understood some details of how this should work. One thing that Jane already mentioned is how it will work with any? Should we strive to have these two side by side or somehow merge them?

view this post on Zulip Nick Cameron (Aug 29 2021 at 01:58):

It's an excellent question! I've thought about it a bit, but as far as I can tell, there isn't a good solution. We can't change any because it's stable and widely used. We can't be closer to any because we are working around some pretty fundamental limitations of it. We could do something simple like nest provide_any inside any (I should add that as an alternative), but I can't think of anything more significant than that.

view this post on Zulip Nick Cameron (Nov 02 2021 at 13:45):

@Jane Lusby [she/her] (and others) I have updated the generic member access RFC to address all the comments: https://github.com/nrc/rfcs/pull/1

view this post on Zulip Nick Cameron (Nov 02 2021 at 13:46):

I've also improved documentation of the prototype implementation: https://github.com/nrc/provide-any

view this post on Zulip Nick Cameron (Nov 02 2021 at 13:47):

Let me know if you have further comments. I think I would like to improve the descriptions in the RFC a little before submitting, but otherwise I think it is good to go

view this post on Zulip Jane Lusby [she/her] (Nov 02 2021 at 13:52):

I'll check it out but I recommend posting it regardless, I can leave comments on the actual PR

view this post on Zulip Ashley Mannix (Nov 04 2021 at 00:02):

https://github.com/nrc/provide-any/blob/main/src/tests.rs#L12-L18

Ah so that's what it's all about. We're basically providing a type-to-value map on-demand that the caller can sift through to find the bit they want and hand back to the original caller.

I'll have a play with this too from a logging context and see what it's like for trying to agree on standards like timestamps, errors, location info etc

view this post on Zulip Jane Lusby [she/her] (Nov 04 2021 at 00:03):

:D

view this post on Zulip Nick Cameron (Nov 04 2021 at 11:31):

RFC posted: https://github.com/rust-lang/rfcs/pull/3192


Last updated: Jan 26 2022 at 14:20 UTC