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

Replace references of static externs to addr_of!. We need to do this … #56

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

safinaskar
Copy link

…because of rust-lang/rust#114447 .

I need this, because I plan to send PR to crate libc, which will add environ static extern for UNIX. Unfortunately, environ generates annoying warning during testing due to rust-lang/rust#114447 , so I'm sending this PR to ctest2 first.

I tested this ctest2 PR with my fork of crate libc, which adds environ, and all works great.

Also, even if you opposed to proposed crate libc change, this ctest2 PR is of general usefulness anyway.

Also, this ctest2 PR always unconditionally generates addr_of! invocation. This is okay, because ctest2's MSRV is 1.56.0, and addr_of! was added in 1.51.0.

Please, make ctest2 release after this PR is applied

@safinaskar
Copy link
Author

CI is failing. But I think this is not because of this PR. If you want, I can try to fix CI anyway

Askar Safin added 2 commits April 20, 2024 23:48
@safinaskar
Copy link
Author

@JohnTitor , I fixed CI!!!

I added two commits (second and third), and now CI works. First commit (i. e. introducing addr_of!) is needed for CI, too. So, all 3 commits are needed for CI. Please, review and apply this PR

Copy link
Owner

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Thanks!

@JohnTitor JohnTitor merged commit 875b40e into JohnTitor:master Jun 25, 2024
9 checks passed
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