parser(refactor): promise only one Source on a thread at a time

[overlookmotel]

Introduce invariant that only a single lexer::Source can exist on a thread at one time.

This is a preparatory step for #2341.

2 notes:

Restriction is only 1 x ParserImpl / Lexer / Source on 1 thread at a time, not globally. So this does not prevent parsing multiple files simultaneously on different threads.

Restriction does not apply to public type Parser, only ParserImpl. ParserImpls are not created in created in Parser::new, but instead in Parser::parse, where they're created and then immediately consumed. So the end user is also free to create multiple Parser instances (if they want to for some reason) on the same thread.

[overlookmotel]

Current dependencies on/for this PR:

• main
  • PR refactor(parser): wrapper type for parser #2339
    • PR parser(refactor): promise only one Source on a thread at a time #2340 👈
      • PR refactor(parser): make Source::set_position safe #2341

This stack of pull requests is managed by Graphite.

[codspeed-hq]

CodSpeed Performance Report

Merging #2340 will not alter performance

_{Comparing 02-07-parser_refactor_promise_only_one_on_a_thread_at_a_time (119501c) with main (0bdecb5)}

Summary

✅ 27 untouched benchmarks

[overlookmotel]

PS: I initially wanted to use the type system to enforce this invariant, rather than making ParserImpl::new, Lexer::new and Source::new unsafe.

This would be possible using a "uniqueness token" type which only Parser::parse can create, and the new methods would require this token as a function parameter. This would be more elegant in my opinion.

However, this would (by design) make it impossible to create a ParserImpl except in Parser::parse, which is incompatible with some of the unit tests, which need to create a ParserImpl and manipulate it manually.

Or is there some way to conditionally expose an extra type/method only when running tests? Something like #[cfg(test)] pub fn test_backdoor()?

[overlookmotel]

Actually, yes, this is possible. Am going to refactor to use types to enforce the invariant, rather than unsafe.

Changed to draft until I've done that.

[Boshen]

Yeah the unsafes are annoying.

Besides, how do we even create a test case for this one?

[overlookmotel]

I've now refactored to enforce this invariant through the type system instead of unsafe.

But have to stop for today now. I'm neglecting my day job!

[overlookmotel]

Yeah the unsafes are annoying.

Besides, how do we even create a test case for this one?

My new type system-based approach removes the unsafe.

I don't think with this new approach it's possible to write tests for it, as there's just no API to do what is meant to be illegal. But that's good, right?

[overlookmotel]

I've changed this PR quite a lot since you first signed off on it @Boshen. Would you mind taking another look now?

Very slight regression on the antd.js parser benchmark (0.5 ms), but I'm hoping we can live with that?

From my side at least, I'm happy with this now. Please merge it if you like it, or tell me if you don't!

[Boshen]

Merge activity

• Feb 8, 1:51 AM EST: @Boshen started a stack merge that includes this pull request via Graphite.
• Feb 8, 1:51 AM EST: @Boshen merged this pull request with Graphite.