Soundness test "fix": work around nightly change of -Zpolonius
#16
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
Fixes the "future-proofing CI" failures caused by a "false positive regression" in nightly rustc's semantics of `-Zpolonius`. This crate tries to justify its usage of `unsafe` by offering a way to disable it when `-Zpolonius` is used, _via_ its Cargo `--feature polonius`. Indeed, when `-Zpolonius` is being used, there is no need to used any of the API of this crate, which, in fact, includes the very implementation of this crate. So we do precisely that, thereby avoiding the need for any `unsafe`, and thus "proving" that our branch API is sound, _even beyond a `-Zpolonius` context_, since it is proven to be sound there. However, around June, the semantics of `-Zpolonius` changed a bit in `rustc` (nightly), and this non-`unsafe`-`-Zpolonius`-vetted implementation was actually being rejected by it. There is, in fact, an issue about this very thing: - rust-lang/rust#126520 It is unclear whether the rejection is fully legitimate; in this case, and also in the minimization I wrote on that issue, it is a false positive: in the non-dependent branch, there is nothing _dependent_ being dropped "after" our `return` "starts". Still, it may be difficult for the compiler maintainers to properly identify and distinguish this legitimate case from one where the drop could have been problematic (in such cases, the change of `-Zpolonius` semantics was actually a soundness fix!). We thus decide to just work around this tiny "false positive regression" issue by making sure our "drop" of the remainders of the non-dependent value (which happen to be a silly empty ZST (`Placeholder`)) clearly happen, syntactically, before our re-access to the initially (re)borrowed value occurs, when we "start" our `return`.
danielhenrymantilla
deleted the
zpolonius-test-nightly-change-workaround
branch
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.
Fixes the "future-proofing CI" failures caused by a "false positive regression" in nightly rustc's semantics of
-Zpolonius.This crate tries to justify its usage of
unsafeby offering a way to disable it when-Zpoloniusis used, via its Cargo--feature polonius.Indeed, when
-Zpoloniusis being used, there is no need to used any of the API of this crate, which, in fact, includes the very implementation of this crate. So we do precisely that, thereby avoiding the need for anyunsafe, and thus "proving" that our branch API is sound, even beyond a-Zpoloniuscontext, since it is proven to be sound there.However, around June, the semantics of
-Zpoloniuschanged a bit inrustc(nightly), and this non-unsafe--Zpolonius-vetted implementation was actually being rejected by it.There is, in fact, an issue about this very thing:
-Zpoloniusrust-lang/rust#126520It is unclear whether the rejection is fully legitimate; in this case, and also in the minimization I wrote on that issue, it is a false positive: in the non-dependent branch, there is nothing dependent being dropped "after" our
return"starts". Still, it may be difficult for the compiler maintainer(s) working on this areä to properly identify and distinguish this legitimate case from one where the drop could have been problematic (in such cases, the change of-Zpoloniussemantics was actually a soundness fix!).We thus decide to just work around this tiny "false positive regression" issue by making sure our "drop" of the remainders of the non-dependent value (which happen to be a silly empty ZST (
Placeholder)) clearly happen, syntactically, before our re-access to the initially (re)borrowed value occurs, when we "start" ourreturn.