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.

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

parser: shrink the size of WithSpan to one register #257

Merged
merged 2 commits into from
Nov 24, 2024

Conversation

Kijewski
Copy link
Collaborator

To be rebased after #214 is merged.

@@ -134,19 +134,70 @@ impl<'a> Ast<'a> {
/// in the code generation.
pub struct WithSpan<'a, T> {
inner: T,
span: &'a str,
span: Span<'a>,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To ensure the size doesn't grow up, might be worth adding a compile-time size_of check of WithSpan and of Span.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it matters too much, as the PR fixes another problem, too:

If the input span is shrunk at the end of the input, e.g. let i = i.trim(), then the error messages will be broken afterward, because only the lengths are compared. This PR fixes that, too. This was actually my original intention with, because I had to refactor some code a few times in order not to break the error messages. That the Span is smaller afterwards is just a happy co-incidence. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a check, anyway. :)

@Kijewski
Copy link
Collaborator Author

Kijewski commented Nov 19, 2024

Oh, I did not expect a fuzzer error, but it makes me happy that it is actually working!

Offset::offset_to only accepts slices of self

Oops, I should have read the documentation of the method more carefully. :)

The failed scenario:

Parser(
    Scenario {
        syntax: Syntax {
            block_start: "{%",
            block_end: "%}",
            expr_start: "{{",
            expr_end: "}}",
            comment_start: "{#",
            comment_end: "#}",
        },
        src: "FFFFF\u{1a}FFFFFFFD& {%\r\rset\rnnnnnnnnnnnyA%}\u{11}\rr\ti-0)mmm..%}68-iioirLLLLLLLLcLLLLLLLLLLLLLL1i2{%for\r\rr\tin\rif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rFFforn\r\rmmm%}68-0)mmm%}68-iioirLLLLLLLLcLLLLLLLLLLLLLL1i2{%for\r\rr\tin\tif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rLLLLLL& {%\r\rset\rnnnnnnnnnnnyA%}\u{11}\rr\ti-0)mmm%}68-iioirLLLLLLLLcLLLLLLLLLLLLLL1i2{%for\r\rr\tin\rif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rFFfom%}68-iioirLLLLLLLLcLLLLLLLLLLLLLL1i2{%for\r\rr\tin\rif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rFFforn\r\rmmm%}68-0)mmm%}68-iioirLLLLLLLLcLLLLLLLLLLLLLL1i2{%for\r\rr\tin\tif*W%}\u{14} s\0\0\0{%-\r\relse+%}|\rLLLLLLLL1i2{%for\r\rr\tin\rif*W%}\u{14}LL1i2{%for\r\rr\tin\rif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rFFforn\r\rmmm%F\rnnnnnnnnLLLLLL1i2{%for\r\rr\tin\tif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rLLLLLLLL1i2{%for\r\rrr\tin\rif*W%}\u{14}LL1i2{%for\r\rr\tin\rif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rFFforn\r\rmmm%F\rnnnnnnnnLLLLLL1i2{%for\r\rr\tin\tif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rLLLLLLLL1i2{%for\r\rr\tin\rif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\riioirLLLLLLLLcLLLLLLLLLLLLLL1i2{%for\r\rr\tin\tif*W%}\u{14} s\0\0\0{%-\r\relse+%}|\rLLLLLLLL1i2{%for\r\rr\tin\r\tin\rif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\riioirLLLLLLLLcLLLLLLLLLLLLLL1i2{%for\r\rr\tin\tif*W%}\u{14} s\0\0\0{%-\r\relse+%}|\rLLLLLLLL1i2{%for\r\rr\tin\rif*W%}\u{14}LL1i2{%for\r\rr\tin\rif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rFFforn\r\rmmm%F\rnnnnnnnnLLLLLL1i2{%for\r\rr\tin\tif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rLLLLLLLL1i2{%for\r\rr\tin\rif*W%}\u{14}  !\u{19} {%-\r\relse+%}|\rFFforn\r\rmmm%F\rnnnnnnnnnnnyA%\r}yyyA%}",
    },
)

@Kijewski Kijewski force-pushed the pr-smaller-span branch 4 times, most recently from 3fc679b to 45c9b54 Compare November 24, 2024 00:39
@Kijewski Kijewski changed the title [WIP] parser: shrink the size of WithSpan to one register parser: shrink the size of WithSpan to one register Nov 24, 2024
@Kijewski Kijewski marked this pull request as ready for review November 24, 2024 21:35
@Kijewski
Copy link
Collaborator Author

Well, the changes in the parser are not too big. No reason to wait.

@GuillaumeGomez
Copy link
Contributor

You even added the fuzzing error, nice!

Well, the changes in the parser are not too big.

As the reviewer, I disagree. :p

@GuillaumeGomez
Copy link
Contributor

Please merge once fuzzer passed.

@Kijewski Kijewski merged commit 87f611e into rinja-rs:master Nov 24, 2024
36 checks passed
@Kijewski Kijewski deleted the pr-smaller-span branch November 24, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants