pchickey opened PR #2428 from pch/wiggle_immut_borrows
to main
:
This PR renames the existing
GuestSlice
andGuestStr
(which impl bothDeref
andDerefMut
) toGuestSliceMut
andGuestStrMut
, and makes an immutable version of each calledGuestSlice
andGuestStr
, which only implDeref
.The borrow checker now distinguishes between mutable and immutable borrows, allowing overlapping immutable borrows, while still requiring mutable borrows be exclusive.
This also moves the borrow checker implementation, which was duplicated under
wiggle/wasmtime
andwiggle/test-helpers
(and additionally over in Lucet), into its own crate. Users can still choose whether to use it toimpl GuestMemory
or not, but putting it in a crate gets rid of some code duplication, which I care about now that the code actually changed.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey requested alexcrichton for a review on PR #2428.
pchickey edited PR #2428 from pch/wiggle_immut_borrows
to main
:
This PR renames the existing
GuestSlice
andGuestStr
(which impl bothDeref
andDerefMut
) toGuestSliceMut
andGuestStrMut
, and makes an immutable version of each calledGuestSlice
andGuestStr
, which only implDeref
.The borrow checker now distinguishes between mutable and immutable borrows, allowing overlapping immutable borrows, while still requiring mutable borrows be exclusive. Tests have been updated to exercise this, and wasi-common has been ported to borrow mutable strs and slices where necessary.
This also moves the borrow checker implementation, which was duplicated under
wiggle/wasmtime
andwiggle/test-helpers
(and additionally over in Lucet), into its own crate. Users can still choose whether to use it toimpl GuestMemory
or not, but putting it in a crate gets rid of some code duplication, which I care about now that the code actually changed.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey edited PR #2428 from pch/wiggle_immut_borrows
to main
:
This PR renames the existing
GuestSlice
andGuestStr
(which impl bothDeref
andDerefMut
) toGuestSliceMut
andGuestStrMut
, and makes an immutable version of each calledGuestSlice
andGuestStr
, which only implDeref
.The borrow checker now distinguishes between mutable and immutable borrows, allowing overlapping immutable borrows, while still requiring mutable borrows be exclusive. Tests have been updated to exercise this, and wasi-common has been ported to borrow mutable strs and slices where necessary.
This also moves the borrow checker implementation, which was duplicated under
wiggle/wasmtime
andwiggle/test-helpers
(and additionally over in Lucet), into its own cratewiggle-borrow
atwiggle/borrow
. Users can still choose whether to use it toimpl GuestMemory
or not, but putting it in a crate gets rid of some code duplication, which I care about now that the code actually changed.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
This looks like it's probably the same as
as_slice_mut
(for the most part), so could some of the internal bits be refactored into a helper called from both functions?
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Perhaps eliding the
!
and switching toany
?
alexcrichton created PR Review Comment:
(same thoughts as above for slices)
alexcrichton created PR Review Comment:
We technically statically know whether this is a shared or mutable borrow, so could there be two unborrow methods instead of one that checks both maps?
alexcrichton created PR Review Comment:
FWIW this is excluded from publication so it's fine to avoid bumping the version here.
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
The comments for
mut_borrow
can now refer toGuestSliceMut
andGuestStrMut
. And this comment can be adapted forimmut_borrow
below to refer toGuestSlice
andGuestStr
,&
access, and to drop the "at most one" language.
sunfishcode created PR Review Comment:
This
unsafe
could use a coment similar to above;SAFETY: iff there are no overlapping borrows it is ok to construct a &mut str
.
sunfishcode created PR Review Comment:
Should this say "no overlapping mut borrows it is ok to construct a &str`?
sunfishcode submitted PR Review.
pchickey updated PR #2428 from pch/wiggle_immut_borrows
to main
:
This PR renames the existing
GuestSlice
andGuestStr
(which impl bothDeref
andDerefMut
) toGuestSliceMut
andGuestStrMut
, and makes an immutable version of each calledGuestSlice
andGuestStr
, which only implDeref
.The borrow checker now distinguishes between mutable and immutable borrows, allowing overlapping immutable borrows, while still requiring mutable borrows be exclusive. Tests have been updated to exercise this, and wasi-common has been ported to borrow mutable strs and slices where necessary.
This also moves the borrow checker implementation, which was duplicated under
wiggle/wasmtime
andwiggle/test-helpers
(and additionally over in Lucet), into its own cratewiggle-borrow
atwiggle/borrow
. Users can still choose whether to use it toimpl GuestMemory
or not, but putting it in a crate gets rid of some code duplication, which I care about now that the code actually changed.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
abrown created PR Review Comment:
:+1: This is a helpful change for users who may want to reuse this script to install OpenVINO; not sure why the paths seem messed up in the CI (I usually use a simpler version of "find the directory this script lives in," FWIW). If you want, you could use Bash variables defaults for the version,
${1:-"..."}
.
abrown submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Is it possible to
debug_assert!
that this did a removal?
pchickey submitted PR Review.
pchickey created PR Review Comment:
I don't know bash very well, this is just a copypaste of what stackoverflow says to do... I'll see what went wrong in CI
pchickey submitted PR Review.
pchickey created PR Review Comment:
Done!
pchickey submitted PR Review.
pchickey created PR Review Comment:
Great idea
pchickey submitted PR Review.
pchickey created PR Review Comment:
I looked into this and because of the interleaving of the mut/shared with validation theres really only opportunity to factor out the top two statements into a func, which I don't think is worth the readability hit.
pchickey updated PR #2428 from pch/wiggle_immut_borrows
to main
:
This PR renames the existing
GuestSlice
andGuestStr
(which impl bothDeref
andDerefMut
) toGuestSliceMut
andGuestStrMut
, and makes an immutable version of each calledGuestSlice
andGuestStr
, which only implDeref
.The borrow checker now distinguishes between mutable and immutable borrows, allowing overlapping immutable borrows, while still requiring mutable borrows be exclusive. Tests have been updated to exercise this, and wasi-common has been ported to borrow mutable strs and slices where necessary.
This also moves the borrow checker implementation, which was duplicated under
wiggle/wasmtime
andwiggle/test-helpers
(and additionally over in Lucet), into its own cratewiggle-borrow
atwiggle/borrow
. Users can still choose whether to use it toimpl GuestMemory
or not, but putting it in a crate gets rid of some code duplication, which I care about now that the code actually changed.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #2428 from pch/wiggle_immut_borrows
to main
:
This PR renames the existing
GuestSlice
andGuestStr
(which impl bothDeref
andDerefMut
) toGuestSliceMut
andGuestStrMut
, and makes an immutable version of each calledGuestSlice
andGuestStr
, which only implDeref
.The borrow checker now distinguishes between mutable and immutable borrows, allowing overlapping immutable borrows, while still requiring mutable borrows be exclusive. Tests have been updated to exercise this, and wasi-common has been ported to borrow mutable strs and slices where necessary.
This also moves the borrow checker implementation, which was duplicated under
wiggle/wasmtime
andwiggle/test-helpers
(and additionally over in Lucet), into its own cratewiggle-borrow
atwiggle/borrow
. Users can still choose whether to use it toimpl GuestMemory
or not, but putting it in a crate gets rid of some code duplication, which I care about now that the code actually changed.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey submitted PR Review.
pchickey created PR Review Comment:
The script error was a simple typo, should be fixed now
pchickey updated PR #2428 from pch/wiggle_immut_borrows
to main
:
This PR renames the existing
GuestSlice
andGuestStr
(which impl bothDeref
andDerefMut
) toGuestSliceMut
andGuestStrMut
, and makes an immutable version of each calledGuestSlice
andGuestStr
, which only implDeref
.The borrow checker now distinguishes between mutable and immutable borrows, allowing overlapping immutable borrows, while still requiring mutable borrows be exclusive. Tests have been updated to exercise this, and wasi-common has been ported to borrow mutable strs and slices where necessary.
This also moves the borrow checker implementation, which was duplicated under
wiggle/wasmtime
andwiggle/test-helpers
(and additionally over in Lucet), into its own cratewiggle-borrow
atwiggle/borrow
. Users can still choose whether to use it toimpl GuestMemory
or not, but putting it in a crate gets rid of some code duplication, which I care about now that the code actually changed.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #2428 from pch/wiggle_immut_borrows
to main
:
This PR renames the existing
GuestSlice
andGuestStr
(which impl bothDeref
andDerefMut
) toGuestSliceMut
andGuestStrMut
, and makes an immutable version of each calledGuestSlice
andGuestStr
, which only implDeref
.The borrow checker now distinguishes between mutable and immutable borrows, allowing overlapping immutable borrows, while still requiring mutable borrows be exclusive. Tests have been updated to exercise this, and wasi-common has been ported to borrow mutable strs and slices where necessary.
This also moves the borrow checker implementation, which was duplicated under
wiggle/wasmtime
andwiggle/test-helpers
(and additionally over in Lucet), into its own cratewiggle-borrow
atwiggle/borrow
. Users can still choose whether to use it toimpl GuestMemory
or not, but putting it in a crate gets rid of some code duplication, which I care about now that the code actually changed.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #2428 from pch/wiggle_immut_borrows
to main
:
This PR renames the existing
GuestSlice
andGuestStr
(which impl bothDeref
andDerefMut
) toGuestSliceMut
andGuestStrMut
, and makes an immutable version of each calledGuestSlice
andGuestStr
, which only implDeref
.The borrow checker now distinguishes between mutable and immutable borrows, allowing overlapping immutable borrows, while still requiring mutable borrows be exclusive. Tests have been updated to exercise this, and wasi-common has been ported to borrow mutable strs and slices where necessary.
This also moves the borrow checker implementation, which was duplicated under
wiggle/wasmtime
andwiggle/test-helpers
(and additionally over in Lucet), into its own cratewiggle-borrow
atwiggle/borrow
. Users can still choose whether to use it toimpl GuestMemory
or not, but putting it in a crate gets rid of some code duplication, which I care about now that the code actually changed.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey merged PR #2428.
Last updated: Dec 23 2024 at 12:05 UTC