search

lukaslueg / macro_railroad

A library to generate syntax diagrams for Rust macros.
MIT License
536 stars 11 forks source link

Generator doesn't simplify diagram in certain cases #22

Closed Osspial closed 5 years ago

Osspial commented 5 years ago

If you generate a syntax diagram for this macro:

macro_rules! isolate_params {
    ($self:ident.$fn:ident(&self $(, $ident:ident: $ty:ty)*)) => ($self.$fn($($ident),*));
    ($self:ident.$fn:ident(&$($lt:tt)? self $(, $ident:ident: $ty:ty)*)) => ($self.$fn($($ident),*));
}

The right-hand part of the diagram doesn't get properly collapsed down, resulting in a diagram that looks like this:

image

If you simplify the macro a bit, the diagram generator behaves correctly. I've removed the $self:ident.$fn:ident prefix here, and the generator behaves as follows:

macro_rules! isolate_params {
    (&self $(, $ident:ident: $ty:ty)*) => ($self.$fn($($ident),*));
    (&$($lt:tt)? self $(, $ident:ident: $ty:ty)*) => ($self.$fn($($ident),*));
}

image

Which is more in line with what I'd expect to see in the first case.

As an aside to this issue: thank you for making this! Having this around has been super helpful for debugging my own macros, and produces output that's much easier to reason about than the trace_macros stuff built into the compiler.

lukaslueg commented 5 years ago

This is actually expected behavior. The dotted boxes on the right side of both paths are Groups as seen by the parser (including rustc), bound by parentheses on both sides. By default, the parser in macro_railroads preserves groups and renders them as in your first example above. Also, by preserving them, the optimizer is not allowed to break them apart while folding common sections; either groups fold because they are identical in their entirety or they don't fold at all. There is some lengthy comment about the ups and downs here.

If you are using the web editor, there is a switch "Preserve groups", which toggles this behaviour. Turned off, both paths in your macro fold completely.

If you are using the library itself, you can call ungroup() on the AST after lowering.

Your second example demonstrates a case where the optimizer could do a better job by removing the nested options. This is caused by the fact that both paths in the second example are actually identical. I'll add some code to do that.

lukaslueg commented 5 years ago

The recent commits fix the aforementioned problem with Repeat::AtMostOnce // Option.