Fix DebugParser.

[nnethercote]

I tried using this and it didn't work at all. prev_token is never eof, so the accumulator is always false, which means the then_some always returns None, which means scan always returns None, and tokens always ends up an empty vec. I'm not sure how this code was supposed to work.

(An aside: I find Iterator::scan to be a pretty wretched function, that produces code which is very hard to understand. Probably why this is just one of two uses of it in the entire compiler.)

This commit changes it to a simpler imperative style that produces a valid tokens vec.

r? @workingjubilee

[workingjubilee]

hmm, I'm quite certain I did test the same code I committed, but it was inside a macro expansion, so it might have been an unusual case... (and lately the specific tokenization of macros may be subtly changing in a way only visible to this, judging by recent commits).

[nnethercote]

I'm not aware of any cases where prev_token becomes Eof. Anyway, any problems with merging this, given that the existing code doesn't work in basic cases?

[nnethercote]

@workingjubilee I'm using debug_lookahead literally right now in collect_tokens_trailing_token. Here's an example. With the old code, I get this:

Parser { prev_token: Token { kind: Question, span: no-location (#0) }, tokens: [], approx_token_stream_pos: 1, .. }

With the new code, I get this:

Parser { prev_token: Token { kind: Question, span: no-location (#0) }, tokens: [Ident("struct", No), Ident("_S", No), OpenDelim(Parenthesis)], approx_token_stream_pos: 1, .. }

A big difference in usefulness!

[workingjubilee]

I was using it on code generated from rustc_expand at the time.

[workingjubilee]

If I recall correctly, what lead to the somewhat awkward formation here is that there were multiple Eof tokens in a row, which I did not understand but also did not try to understand.

Insofar as an "objection", I would prefer to conduct a modest amount of investigation into why the current state of affairs is, er, the current state of affairs, and then r+ this, because "basic" is relative and I wouldn't want to make this useless on what I was doing. But it requires retracing my steps, and from about Thursday to now I was completely exhausted and overheated so I did not get much serious thought done. If you can find out why, based on the scant description, I made the mistake I did (if it was a mistake?), then I am happy to accept something plausible, otherwise I will try to get to it shortly.

[nnethercote]

You can get multiple Eofs if you look past the end of the tokenstream. Something weird would have to be happening.

[workingjubilee]

I see.

I suppose we are filing "macro expansion" under "something weird"?

[nnethercote]

I suppose we are filing "macro expansion" under "something weird"?

I don't know how you were using it. I do know that the existing code doesn't work for my simple use case, and the new code does.

If I wrote a test or two would that help?

[workingjubilee]

Yes, please. I really should have committed some, but I didn't expect this to be this complicated, and it seemed contra the "debug use only" thing... 😅

[nnethercote]

New code is up, with tests showing before and after behaviour.

[workingjubilee]

Thanks!

I think I got direly confused at some point by hitting some of the "quirks" in the special case of before, and that plus inexperience with the parser (or indeed reasoning about tokenstreams) resulted in the current state of things.

@bors r+

[bors]

📌 Commit aa0e8e1 has been approved by workingjubilee

It is now in the queue for this repository.

[nnethercote]

@bors rollup