Seal traits inside impl_ submodule

[davidhewitt]

Off the back of #3897 (comment)

Should we review the traits inside the impl_ submodule, check which ones can be sealed, and do that? It's a reasonable safety mechanism to prevent users from trying to implement those things.

[davidhewitt]

I labelled this "Good First Issue", as this is an internal refactoring which would be great to have.

A "sealed" trait has a private supertrait, thus making it possible to implement outside the crate. All of the PyAnyMethods and similar traits are sealed, for example. Take a look at src/sealed.rs as the trait we use as the supertrait for most sealing.

[Cheukting]

@davidhewitt I am happy to have a look at this

[davidhewitt]

Brilliant, thanks 🙏

[Cheukting]

@davidhewitt quick question, are there any particular reasons something should be or should not be moved? Or is it ideally everything should be moved from impl_ to Sealed? I want to make sure I fully understand this.

[davidhewitt]

I don't think we should be moving any code, this issue is about upgrading trait Foo to trait Foo: Sealed where it makes sense to do so for the traits inside the impl_ submodule.

[Cheukting]

I see, so those traits that could be used by the user need to be Sealed right? Thanks for clarifying it @davidhewitt

[davidhewitt]

Yes, in principle many things inside impl_ are accessible by users but we never intend them to use them directly (just via the macros). The worry is there might be traits in there which we would prefer users can't implement at all, hence the sealing.

[Cheukting]

Thank you for your patience, I was occupied by EuroPython preparation in the last few weeks. I plan to pick this one up again this weekend or next week.

[davidhewitt]

I think the next step along here would be finding traits which make use of the private_impl! helper and changing them all to be Sealed instead (and remove the old helper).

[Cheukting]

@davidhewitt @LilyFoote is this issue closed? It seems so to me

[davidhewitt]

I think there are still lots of traits in impl_ (search for pub trait) which do not have a Sealed bound. So it looks like there is more which can be done.

[Cheukting]

Cool, I will go treasure hunt when I got the time!