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

rustc is now making use of the fact that derefencing a null pointer is UB. Remove all null dereferences #1209

Merged
merged 1 commit into from Jul 1, 2019

Conversation

rukai
Copy link
Member

@rukai rukai commented Jun 25, 2019

  • Added an entry to CHANGELOG_VULKANO.md or CHANGELOG_VK_SYS.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes - in this repository
  • Updated documentation to reflect any user-facing changes - PR to the guide that fixes existing documentation invalidated by this PR.

Uh oh..
rustc is now making use of the fact that derefencing a null pointer is UB.
So we need to remove all null dereferences, of which there are surprisingly large amount of. In order to get vulkano working AT ALL on nightly and presumably soon beta + stable.

@tomaka You have a pretty good idea what is and isnt UB. Why did you write these null dereferences, is there something you needed from them that I am missing?

Fixes #1207

@rukai
Copy link
Member Author

rukai commented Jun 25, 2019

Ok, this PR now runs on the latest nightly.

Unfortunately I could not find a way to do so without a breaking change.
The vertex struct provided by the user now needs to #[derive(Default)]
If anyone can provide a fix that isn't breaking I will gladly take it!

I'll wait a few days to get feedback from people. @mitchmindtree @freesig
Then I'll go ahead and release this as a breaking change so people can transition before vulkano breaks on stable rust.

@tomaka
Copy link
Member

tomaka commented Jun 25, 2019

It was a way to be sure that the offset is actually correct. In other words, a replacement for the offsetof! macro that Rust is lacking.

@rukai
Copy link
Member Author

rukai commented Jun 25, 2019

Is the variable called size poorly named and should really be another type of offset?

I just replaced the size stuff with size_of which worked on everything I tested, but might still be wrong?

@tomaka
Copy link
Member

tomaka commented Jun 25, 2019

I guess size_of should work, but is not necessarily 100% future-proof. In practice it should be fine, I guess.

@rukai rukai merged commit f611932 into vulkano-rs:master Jul 1, 2019
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.

Shader compilation fails with: "No Rust equivalent for a floating-point of width 32"
2 participants