fix: fix assertion failed panic when input is # - #22873
Conversation
rustbot
added
the
S-waiting-on-review
| pub(crate) fn expr(p: &mut Parser<'_>) { | ||
| let m = p.start(); | ||
| expressions::expr(p); | ||
| let starts_with_attr = p.at(T![#]); |
There was a problem hiding this comment.
This function is fine and should not be changed. The change is also wrong - consider e.g. #[attr] 1. The change should probably be in expression::expr(), but I haven't thought where exactly.
|
@rustbot ready |
| r: Restrictions, | ||
| bp: u8, | ||
| ) -> Option<(CompletedMarker, BlockLike)> { | ||
| let mut starts_with_attr = false; |
There was a problem hiding this comment.
It seems you continue to misunderstand. There's nothing wrong with an expr that starts with #. Therefore it's not what matters and not what we should look at. I'm not sure if just changing m.abandon(p) to m.complete(ERROR) will cause problems, but it's definitely in a better shape.
There was a problem hiding this comment.
I remove the fix code and replace m.abandon(p) to m.complete(ERROR) which failed two tests:macro_expansion_tests::builtin_fn_macro::regression_15002
(result: builtin #format_args ("{}", x =; expect:builtin #format_args ("{}", x = );),
macro_expansion_tests::mbe::regression::eager_regression_154032
(result:builtin #format_args ("{}", &[0 2; expect:builtin #format_args ("{}", &[0 2]);)
so in some cases we still need m.abandon(p), maybe change the condition to check whether outer_attrs(p) actually consumes an attribute?
There was a problem hiding this comment.
No. The way I proposed is the beginning of a good way, the way of checking # (including checking outer_attrs()) is a dead end. The next thing to check is why the tests fail.
There was a problem hiding this comment.
There are two reasons why tests failed.
- when the error recovering is succeeded,
m.complete(ERROR)would change the intended recovery shape and cause following tokens to be handled incorrectly by the outer parser layer.
(test case:macro_expansion_tests::builtin_fn_macro::regression_15002) - when marker is empty,
m.complete(ERROR)would add aERRORnode which form a nestERRORnode.
(test case:tests::parse_err)
I think both the cases should usem.abandon(p)instead ofm.complete(ERROR)
add helper function is_empty() add new condition check
3 participants
fixes #22807
it is because that expressions::expr(p) first consumes # as the start of an attribute.
Since there is no following expression, it returns None.
But the outer marker is abandoned, leading to an invalid top-level parse shape which cause the panic.
So I think it is better to complete a Err Node rather than abondoning.