Fix proc macro panicked with Invalid type in const_random! in Rust 1.77.
#33
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
dtolnay
suggested changes
Jan 26, 2024
There was a problem hiding this comment.
I believe the usize/isize change is not correct, because it assumes the host's pointer size is at least as large as the target's pointer size. If usize is 4 bytes on the host and 8 bytes on the target, const_random would be producing random usizes that are half zero.
nnethercote
force-pushed
the
fix-const_random-for-1.77
branch
2 times, most recently
from
fd239f9
to
025c9c5
Compare
|
Well spotted, thanks. I have updated to remove the first commit changing |
dtolnay
reviewed
Jan 26, 2024
…ust 1.77.
With Rust nightly 1.77, any invocation of `const_random!` with a `u8`
array causes a compile-time panic. This can be seen when running `cargo
test`:
```
error: proc macro panicked
--> tests/tests.rs:52:28
|
52 | const VALUE1: &[u8] = &const_random!([u8; 30]);
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= help: message: Invalid type
```
This is because the proc macro starts by calling `to_string` on the
input token stream, and then uses substring matching to "parse" it. In
Rust 1.77 the `Display` impl for `TokenStream` has changed, and what
used to be converted to the string `"[u8 ; 30]"` is now `"[u8; 30]"`. As
a result, the `byte_array.starts_with("[u8 ; ")` call fails.
Note that substring matching is inherently flawed because the whitespace
in the output of `to_string` is not guaranteed.
This commit rewrites the proc macro to be robust in the face of
`to_string` whitespace changes, by iterating over the individual
`TokenTrees`s.
The commit also adds a comment explaining why `usize` and `isize` are
handled differently, because it's subtle.
Note: I ran `cargo fmt` within `macro/` to format the changes to
`macro/src/lib.rs` and it made some minor changes to `macro/src/span.rs`
as well.
nnethercote
force-pushed
the
fix-const_random-for-1.77
branch
from
025c9c5
to
618d64c
Compare
dtolnay
approved these changes
Jan 26, 2024
|
Oh, this problem may also occur in Rust 1.76. |
4 tasks
tkaitchuck
approved these changes
Mar 3, 2024
|
Would it be possible to get a new version of const-random-macros released with this fix? |
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.
This PR fixes a compile-time panic with
const_random!on byte arrays in Rust 1.77. Full details are in the individual commit logs.