Implement a #[vtable] macro

[nbdd0121]

Use a single #[vtable] macro to replace the current boilerplating of ToUse, USE_NONE, declare_file_operations` required for declaring and implementing traits that maps to Linux's pure vtables and contains optional methods.

This'll be more natural and less noisy than the current approach, and it paves the way for other vtables such as the one in #208 so that we don't need a ToUse, USE_NONE and a macro for each vtable trait.

Little note that this just scans the tokens so cannot handle case like

#[vtable]
impl Foo for Bar<{1 + 1}> {}

but this is very rare. I think we can live with this for now; we can always migrate to proper parsing later if we ever get syn vendored.

[ksquirrel]

Review of a5003f241719:

• ✔️ Commit a5003f2: Looks fine!

[nbdd0121]

I see this pattern in #264 again so we definitely want a single macro abstraction for all vtables.

[wedsonaf]

As you know, this is something that I wanted to do for file_operations and I do think it is more usable than what we have, however:

1. We only really need this when the kernel behaves differently depending on the presence/absence (being null/non-null) of a function pointer. This is the case for file operations: we can't always replicate the kernel behaviour in our default implementations. I'm not sure how prevalent this is, we'd have to analyse it on a case-by-base basis. When we can easily replicate kernel behaviour with our default handlers though, it is much simpler to just use a regular trait and always generate non-null function pointers. No need for macros (procedural or otherwise). Do you know if Add SPI Abstraction #264 really need this?
2. In the file_operations case, a single (generic) function may result in more than one function function. See for example read and write -- these functions can populate the read/read_iter and write/write_iter function pointers at the programmer's choosing: they may not always want both implemented. How does your macro handle this?
3. I'm sorry to say this, but at the moment I think we're better off with the existing implementation. It's much simpler to follow than a procedural macro. In my view, the gains in usability ATM do not outweigh the added complexity.

[nbdd0121]

1. We only really need this when the kernel behaves differently depending on the presence/absence (being null/non-null) of a function pointer. This is the case for file operations: we can't always replicate the kernel behaviour in our default implementations. I'm not sure how prevalent this is, we'd have to analyse it on a case-by-base basis. When we can easily replicate kernel behaviour with our default handlers though, it is much simpler to just use a regular trait and always generate non-null function pointers. No need for macros (procedural or otherwise). Do you know if Add SPI Abstraction #264 really need this?

I don't know if #264 need this, but #208 certainly do.

2. In the file_operations case, a single (generic) function may result in more than one function function. See for example read and write -- these functions can populate the read/read_iter and write/write_iter function pointers at the programmer's choosing: they may not always want both implemented. How does your macro handle this?

Are there reasons not to implement read_iter/write_iter? The current trait design essentially guarantees that it either both or none are supported. Without specialization, it makes no sense to support one but not another. If we really want to let the implementor decide, then generics should not be used.

If we do want them to opt out (rather than opt in), we can just add a manual const HAS_READ_ITER: bool = true; and ask the implementor to define const HAS_READ_ITER: bool = false; to opt out. This way it makes it clear that they do not want to have read_iter/write_iter instead of forgetting to putting them in the declaration.

3. I'm sorry to say this, but at the moment I think we're better off with the existing implementation. It's much simpler to follow than a procedural macro. In my view, the gains in usability ATM do not outweigh the added complexity.

I would suggest the opposite. I would also argue that it is also less complex than the current declarative macro. I got confused when I first saw the ToUse type, const and macro; it is much simpler to say, put on the #[vtable] attribute when you implement this. If people want to know why it is necessary, the vtable macro description provides a central source of information about the design choice, not a dozen of macros each doing exactly the same thing.

This procedural macro is simpler to use, less error prone, less code, less boilerplate, and generate a good documentation. 60 lines of boilerplate for each vtable and hours of debugging caused by forgetting to add the name of method to the declare_*! macro, vs. 80 lines of proc macro, I think I am strongly in favor of the latter.

[nbdd0121]

I don't understand why you're so against proc macros, but I want to make one point: any complexities in proc macros are used to reduce complexities elsewhere. As long as the reduced complexities outweigh the introduced complexities, we should definitely go for it.

As for this case, given that there we are going to have at least 2 (maybe 3) trait users and a couple impl users of this macro, and certainly more in the future, I believe the macro certainly reduces the overall complexity.

[kloenk]

line 66 adds a doc comment about the const value, which is missing in this line.

[nbdd0121]

It's intentional to have doc comment only in trait but not in impls.

[kloenk]

Than I misread. Then nevermind

[wedsonaf]

nit: are you including the prelude just for vtable? If so, I think it's better to add macros::vtable to the list below.

[nbdd0121]

I think it should generally be fine to include prelude. I could remove the Error and Result import so that this import is used more than just vtable.

[wedsonaf]

Even if you want to include prelude::*, you should add it to the line below for consistency.

Now, given the choice of adding macros::vtable or prelude::*, I think the former is preferable because we don't pollute the namespace with more symbols than needed. (prelude is meant for drivers.)

You're obviously welcome to disagree with our handling or prelude, but in that case I'd argue that you're unnecessarily conflating issues: let's open another PR or issue to discuss that if you'd like. I'm ready to merge this once we come to an agreement on the use clause.

[wedsonaf]

Thank you, @nbdd0121!