chore(deps): drop dependency on lexical
#37
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `lexical` crate maintenance status has been called into question while being affected by several soundness issues, as explained in [`RUSTSEC-2023-0055`](https://rustsec.org/advisories/RUSTSEC-2023-0055). However, as far as I can see, we don't really need to use `lexical` in the GLSL preprocessor: - For parsing `#version` numbers, `lexical` didn't provide results that complied with my interpretation of the GLSL specification, which I explained in a code comment, and that I believe motivated a TODO item to move on from `lexical`. Moreover, as `#version` directives are expected to be infrequent (usually, at most one per translation unit), I don't think it's worth to bloat our dependencies with `atoi(_simd|_radix10)`, as `#version` numbers are tiny 16-bit integers where SIMD and bitwise optimizations have a neglible performance impact, so these changes go for correctness and use the INT and UINT token parsing routines to parse these. - For parsing floating point numbers, modern Rust stdlib versions ship a performant algorithm proposed by `lexical`'s author, which negate previous performance gaps between both implementations. Therefore, just use the standard parsing functions in these cases. As far as I am aware, these changes are not semver-breaking because the modified `VersionError` enum was never exposed outside of the `lang-pp` crate.
This is required by a transitive `cargo-readme` dependency.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
lexicalcrate maintenance status has been called into question while being affected by several soundness issues, as explained inRUSTSEC-2023-0055. However, as far as I can see, we don't really need to uselexicalin the GLSL preprocessor:#versionnumbers,lexicaldidn't provide results that complied with my interpretation of the GLSL specification, which I explained in a code comment, and that I believe motivated a TODO item to move on fromlexical. Moreover, as#versiondirectives are expected to be infrequent (usually, at most once per translation unit), I don't think it's worth to bloat our dependencies withatoi(_simd|_radix10), as#versionnumbers are tiny 16-bit integers where SIMD and bitwise optimizations have a neglible performance impact, so these changes go for simplicity and use the INT and UINT token parsing routines to parse these.lexical's author, which negates previous performance gaps between both implementations. Therefore, just use the standard parsing functions in these cases.As far as I am aware, these changes are not semver-breaking because the modified
VersionErrorenum was never exposed outside of thelang-ppcrate.I'm also working on a PR to update all the dependencies to their latest versions, including
syn,logosandlalrpop, to bring these crates up to the most used dependency versions in the ecosystem and reduce build time and dependency tree size. However, these updates are a bit more involved (I ran into issues like TedDriggs/darling#238 with thelang_util::Tokenderive macro), so I'll be completing the updates over the next days.