Rewrite macro_rules! parser to not use the MBE engine itself
#143070
Conversation
|
rustbot has assigned @compiler-errors. Use |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
The `macro_rules!` parser was written to match the series of rules using the macros-by-example (MBE) engine and a hand-written equivalent of the left-hand side of a MBE macro. This was complex to read, difficult to extend, and produced confusing error messages. Because it was using the MBE engine, any parse failure would be reported as if some macro was being applied to the `macro_rules!` invocation itself; for instance, errors would talk about "macro invocation", "macro arguments", and "macro call", when they were actually about the macro *definition*. And in practice, the `macro_rules!` parser only used the MBE engine to extract the left-hand side and right-hand side of each rule as a token tree, and then parsed the rest using a separate parser. Rewrite it to parse the series of rules using a simple loop, instead. This makes it more extensible in the future, and improves error messages. For instance, omitting a semicolon between rules will result in "expected `;`" and "unexpected token", rather than the confusing "no rules expected this token in macro call". This work was greatly aided by pair programming with Vincenzo Palazzo and Eric Holk.
joshtriplett
force-pushed
the
macro-rules-parse
branch
from
5c6db7b to
0776082
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
Rewrite `macro_rules!` parser to not use the MBE engine itself The `macro_rules!` parser was written to match the series of rules using the macros-by-example (MBE) engine and a hand-written equivalent of the left-hand side of a MBE macro. This was complex to read, difficult to extend, and produced confusing error messages. Because it was using the MBE engine, any parse failure would be reported as if some macro was being applied to the `macro_rules!` invocation itself; for instance, errors would talk about "macro invocation", "macro arguments", and "macro call", when they were actually about the macro *definition*. And in practice, the `macro_rules!` parser only used the MBE engine to extract the left-hand side and right-hand side of each rule as a token tree, and then parsed the rest using a separate parser. Rewrite it to parse the series of rules using a simple loop, instead. This makes it more extensible in the future, and improves error messages. For instance, omitting a semicolon between rules will result in "expected `;`" and "unexpected token", rather than the confusing "no rules expected this token in macro call". This work was greatly aided by pair programming with Vincenzo Palazzo (`@vincenzopalazzo)` and Eric Holk (`@eholk).`
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (8b198f2): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 1.4%, secondary 2.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResults (primary 0.0%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 689.258s -> 691.128s (0.27%) |
rustbot
added
perf-regression
|
r? @petrochenkov |
bors
added
S-waiting-on-bors
The
macro_rules!parser was written to match the series of rules using the macros-by-example (MBE) engine and a hand-written equivalent of the left-hand side of a MBE macro. This was complex to read, difficult to extend, and produced confusing error messages. Because it was using the MBE engine, any parse failure would be reported as if some macro was being applied to themacro_rules!invocation itself; for instance, errors would talk about "macro invocation", "macro arguments", and "macro call", when they were actually about the macro definition.And in practice, the
macro_rules!parser only used the MBE engine to extract the left-hand side and right-hand side of each rule as a token tree, and then parsed the rest using a separate parser.Rewrite it to parse the series of rules using a simple loop, instead. This makes it more extensible in the future, and improves error messages. For instance, omitting a semicolon between rules will result in "expected
;" and "unexpected token", rather than the confusing "no rules expected this token in macro call".This work was greatly aided by pair programming with Vincenzo Palazzo (@vincenzopalazzo) and Eric Holk (@eholk).
For review, I recommend reading the two commits separately.