Skip to content

fix(codegen/regex): allow vec growth on parse#405

Merged
jeertmans merged 6 commits intomaciejhirsz:masterfrom
tristan-f-r:issue-389
Aug 8, 2024
Merged

fix(codegen/regex): allow vec growth on parse#405
jeertmans merged 6 commits intomaciejhirsz:masterfrom
tristan-f-r:issue-389

Conversation

@tristan-f-r
Copy link
Contributor

@tristan-f-r tristan-f-r commented Jul 24, 2024

Fixes #389. This is less efficient, but it's on codegen.

@jeertmans
Copy link
Collaborator

Hello @LeoDog896, thank you for your contribution!

Can you motivate why we need to have a possibly growing vector instead?
I see where the error might come from, but that would be great to document / explain it a little better, as it goes against the previous code comments left by the author of Logos.

@tristan-f-r
Copy link
Contributor Author

tristan-f-r commented Jul 25, 2024

Yep! I'll write this in a comment soon, but the original vec allocation never accounted for the size of the literals - it always assumed every ropable element would only have one item, and each item would be four Unicode codepoints long; since literals can be longer than four Unicode points, if there was more than eight tokens (two being the minimum size to create a concat literal), it would cause that integer overflow error.

The only way to get that size would be to walk through all the elements in the concat node and add up the lengths of all of these elements.

@jeertmans
Copy link
Collaborator

Sorry for the late review @LeoDog896, I was on holidays. This looks great and clear to me.

Before I merge this, did you check that long_concat_389 fails without your fix?

@jeertmans jeertmans added the bug Something isn't working label Aug 7, 2024
@tristan-f-r
Copy link
Contributor Author

Sorry for the late review @LeoDog896, I was on holidays. This looks great and clear to me.

Before I merge this, did you check that long_concat_389 fails without your fix?

image

On current master, with only long_concat_389 added, it fails 👍

@jeertmans
Copy link
Collaborator

Looks good, thanks!

@jeertmans jeertmans merged commit 0ce5806 into maciejhirsz:master Aug 8, 2024
akrantz01 referenced this pull request in akrantz01/antsi Sep 19, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [logos](https://logos.maciej.codes/)
([source](https://redirect.github.com/maciejhirsz/logos)) | dependencies
| patch | `0.14.1` -> `0.14.2` |

---

### Release Notes

<details>
<summary>maciejhirsz/logos (logos)</summary>

###
[`v0.14.2`](https://redirect.github.com/maciejhirsz/logos/releases/tag/v0.14.2):
- Optional `forbid_unsafe` feature, fuzzing, book, and more!

[Compare
Source](https://redirect.github.com/maciejhirsz/logos/compare/v0.14.1...v0.14.2)

#### What's Changed

- chore(book): added link to Rust's reference by
[@&#8203;CommanderStorm](https://redirect.github.com/CommanderStorm) in
[https://github.com/maciejhirsz/logos/pull/411](https://redirect.github.com/maciejhirsz/logos/pull/411)
- feat: impl Source for T: Deref in no_std by
[@&#8203;yjhmelody](https://redirect.github.com/yjhmelody) in
[https://github.com/maciejhirsz/logos/pull/406](https://redirect.github.com/maciejhirsz/logos/pull/406)
- fix(codegen/regex): allow vec growth on parse by
[@&#8203;LeoDog896](https://redirect.github.com/LeoDog896) in
[https://github.com/maciejhirsz/logos/pull/405](https://redirect.github.com/maciejhirsz/logos/pull/405)
- test: basic fuzzing by
[@&#8203;LeoDog896](https://redirect.github.com/LeoDog896) in
[https://github.com/maciejhirsz/logos/pull/407](https://redirect.github.com/maciejhirsz/logos/pull/407)
- feat(lib): add `forbid_unsafe` feature to disable unsafe code by
[@&#8203;davidkern](https://redirect.github.com/davidkern) in
[https://github.com/maciejhirsz/logos/pull/413](https://redirect.github.com/maciejhirsz/logos/pull/413)
- chore(version): release v0.14.2 by
[@&#8203;jeertmans](https://redirect.github.com/jeertmans) in
[https://github.com/maciejhirsz/logos/pull/422](https://redirect.github.com/maciejhirsz/logos/pull/422)

#### New Contributors

- [@&#8203;CommanderStorm](https://redirect.github.com/CommanderStorm)
made their first contribution in
[https://github.com/maciejhirsz/logos/pull/411](https://redirect.github.com/maciejhirsz/logos/pull/411)
- [@&#8203;yjhmelody](https://redirect.github.com/yjhmelody) made their
first contribution in
[https://github.com/maciejhirsz/logos/pull/406](https://redirect.github.com/maciejhirsz/logos/pull/406)
- [@&#8203;davidkern](https://redirect.github.com/davidkern) made their
first contribution in
[https://github.com/maciejhirsz/logos/pull/413](https://redirect.github.com/maciejhirsz/logos/pull/413)

**Full Changelog**:
maciejhirsz/logos@v0.14.1...v0.14.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/akrantz01/antsi).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44MC4wIiwidXBkYXRlZEluVmVyIjoiMzguODAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

proc-macro derive panicked message: attempt to subtract with overflow

2 participants