Implement Folder #85
Implement Folder #85
Conversation
|
Thanks! This is exactly what I wanted. I don't have time to review today but I will get to it tomorrow. |
|
@dtolnay I think we should strive to provide a |
|
@gnzlbg I think that a The main reason why I think This is all up to @dtolnay though - so if he thinks that |
|
@mystor I haven't written So... I am a bit stumbled about how to proceed. I don't know enough about macro expansion to know if even attempting to write it as a Even if we were to implement it as a |
|
I don't think I would be in favor of having |
| /// If you want to ensure that your code handles every variant explicitly, you | ||
| /// need to override each method and monitor future changes to `Folder` in case | ||
| /// a new method with a new default implementation gets introduced. | ||
| pub trait Folder: Sized { |
There was a problem hiding this comment.
We don't need this Sized bound. Unclear why libsyntax has it.
There was a problem hiding this comment.
syn::Visitor has this as well, without it, I cannot call any of the noop_ functions in the default methods.
There was a problem hiding this comment.
I think this is just a matter of accepting <F: ?Sized + Folder> in the noop functions. I will see if I can make it work.
| fn fold_ty_param_bound(&mut self, bound: TyParamBound) -> TyParamBound { | ||
| noop_fold_ty_param_bound(self, bound) | ||
| } | ||
| fn fold_poly_trait_ref(&mut self, |
There was a problem hiding this comment.
Let's take only the PolyTraitRef as input here, the way libsyntax does.
| -> PolyTraitRef { | ||
| noop_fold_poly_trait_ref(self, trait_ref, modifier) | ||
| } | ||
| fn fold_variant_data(&mut self, |
There was a problem hiding this comment.
Let's take only the VariantData as input.
| fn fold_field(&mut self, field: Field) -> Field { | ||
| noop_fold_field(self, field) | ||
| } | ||
| fn fold_variant(&mut self, variant: Variant, generics: Generics) -> Variant { |
There was a problem hiding this comment.
Let's take only the Variant as input.
| noop_fold_stmt(self, stmt) | ||
| } | ||
|
|
||
| /// Prefer overriding fold_stmt instead! |
There was a problem hiding this comment.
Why? Block shows up in lots of other places.
There was a problem hiding this comment.
I think one often wants to fold over a Block, so Folder should offer a .fold_block method, but since a Block is just a vector of Stmts, one typically wants to override fold_stmt instead (and not fold_block).
|
@dtolnay I've fixed all issues you mentioned and rebased the PR :) |
Closes #43 .
Adds a new feature: "fold".
The
Foldertrait allows implementing AST->AST transformations.If the way it is implemented looks good, adding the documentation is trivial, but adding some tests will take some work.