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

Fix proc macro panicked with Invalid type in const_random! in Rust 1.77. #33

Merged

Conversation

nnethercote
Copy link
Contributor

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.

Copy link
Contributor

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

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 nnethercote force-pushed the fix-const_random-for-1.77 branch 2 times, most recently from fd239f9 to 025c9c5 Compare January 26, 2024 04:18
@nnethercote
Copy link
Contributor Author

Well spotted, thanks. I have updated to remove the first commit changing usize/isize handling. I also added a comment explaining the subtlety, so nobody else makes the same mistake in the future.

macro/src/lib.rs Outdated Show resolved Hide resolved
…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.
Copy link
Contributor

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thank you! This looks good to me.

@nnethercote
Copy link
Contributor Author

Oh, this problem may also occur in Rust 1.76.

@tkaitchuck tkaitchuck merged commit 4715dd1 into tkaitchuck:master Mar 3, 2024
@nnethercote nnethercote deleted the fix-const_random-for-1.77 branch March 3, 2024 22:29
@decathorpe
Copy link

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants