syn icon indicating copy to clipboard operation
syn copied to clipboard

`parse` should take a `&mut ParseStream`

Open rambip opened this issue 1 year ago • 2 comments

Maybe there is an obvious reason, but I was not able to find any justification, so I will ask my question here. Forgive me if it is naive.

Why does the parse function in the trait Parse take an immutable reference to a ParseStream ? I had to read the docs for hours before understanding that each time you call parse, the stream mutates (It is never explained in the examples).

Since the parse function really mutates its input, I think it would be clearer to change its signature ... And since the user entry-point is parse_macro_input!, I don't think it would change a lot of things for the user.

rambip avatar Jul 08 '23 18:07 rambip

Maybe I am misunderstanding, but if I understand correctly, ParseStream doesn't "mutate" what it refers to, see the docs here (recall that ParseStream is a an alias for ParseBuffer) and the code here.

So, the key points are:

  1. ParseStream never mutates its input TokenStream, it is just a cursor on the input TokenStream.

  2. ParseStream does have interior mutability by the Cell<Cursor>, this is only in order to keep track of where it has read to.

bzm3r avatar Jul 11 '23 05:07 bzm3r

I think you understood correctly. But even if the ParseStream don't mutate the stream, it mutates the cursor, which is unexpected for the user !

Commonly, rust parsing library use Parser traits, and these traits implement a function taking a &mut ref

See here for instance: even if Expr is not mutable, Visitor is. This pattern feels way more intuitive for me

rambip avatar Jul 11 '23 10:07 rambip

For anyone wondering about this, it would be an informative exercise to attempt to produce a compilable PR in which parser functions take &mut. I am confident you will not get very far before realizing it ends up being enormously detrimental to usability.

dtolnay avatar Apr 17 '24 20:04 dtolnay