Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify MatcherPos some more #95509

Merged
merged 5 commits into from
Apr 2, 2022
Merged

Simplify MatcherPos some more #95509

merged 5 commits into from
Apr 2, 2022

Commits on Mar 31, 2022

  1. Simplify exit of Delimited submatchers. 

    Currently, we detect an exit from a `Delimited` submatcher when `idx`
exceeds the bounds of the current submatcher *and* there is a `stack`
entry.

This commit changes it to something simpler: just look for a
`CloseDelim` token.
    @nnethercote
    nnethercote committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    21699c4 View commit details
    Browse the repository at this point in the history

  2. Remove MatcherPos::match_lo. 

    It's redundant w.r.t. other fields.
    @nnethercote
    nnethercote committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    2e423c7 View commit details
    Browse the repository at this point in the history

  3. Clarify idx handling in sequences. 

    By adding comments, and improving an assertion. I finally fully
understand this part!
    @nnethercote
    nnethercote committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    048bd67 View commit details
    Browse the repository at this point in the history

  4. Remove MatcherPos::stack. 

    `parse_tt` needs a way to get from within submatchers make to the
enclosing submatchers. Currently it has two distinct mechanisms for
this:
- `Delimited` submatchers use `MatcherPos::stack` to record stuff about
  the parent (and further back ancestors).
- `Sequence` submatchers use `MatcherPosSequence::parent` to point to
  the parent matcher position.

Having two mechanisms is really confusing, and it took me a long time to
understand all this.

This commit eliminates `MatcherPos::stack`, and changes `Delimited`
submatchers to use the same mechanism as sequence submatchers. That
mechanism is also changed a bit: instead of storing the entire parent
`MatcherPos`, we now only store the necessary parts from the parent
`MatcherPos`.

Overall this is a small performance win, with the positives outweighing
the negatives, but it's mostly for clarity.
    @nnethercote
    nnethercote committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    f68a044 View commit details
    Browse the repository at this point in the history

  5. Make MatcherPos not derive Clone. 

    It's only used in one place, and there we clone and then make a bunch of
modifications. It's clearer if we duplicate more explicitly, and there's
a symmetry now between `sequence()` and `empty_sequence()`.
    @nnethercote
    nnethercote committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    c6fedd4 View commit details
    Browse the repository at this point in the history