Oh, and of course yours suffers from the other limitation which is that you can't apply two such things in the same scope, which is often very useful.
1 Like
You can apply two of these at once. I'd like it if you could at least attempt to meet your needs before requesting an addition to the language.
Is the only missing property that you want to be able to write self.explode_mode instead of |self| &mut self.explode_mode? There's no difference in the behavior here, and you can solve the syntactic overhead with a macro.
1 Like
extern crate scopeguard; // 1.1.0
pub struct Foo {
exploder_mode: bool,
// other fields
}
fn reset(this: &mut Foo) {
this.exploder_mode = false;
}
impl Foo {
pub fn some_call(&mut self) {
self.other_call();
{
let mut guard = scopeguard::guard(&mut *self, reset);
let this: &mut Self = &mut guard;
this.other_call(); // works
// self.other_call(); // does not work; cannot work with the current borrow model
}
self.other_call();
}
pub fn other_call(&mut self) {}
}
Or you can use my janitor! macro upthread to create specific types.
Not true. Just stack them normally!
extern crate scopeguard; // 1.1.0
pub struct Foo {
exploder_mode: bool,
// other fields
}
fn reset(this: &mut Foo) {
this.exploder_mode = false;
}
impl Foo {
pub fn some_call(&mut self) {
self.other_call();
{
let mut guard = scopeguard::guard(&mut *self, reset);
let this: &mut Self = &mut guard;
let mut guard = scopeguard::guard(&mut *this, reset);
let this: &mut Self = &mut guard;
this.other_call(); // works
// self.other_call(); // does not work; cannot work with the current borrow model
}
self.other_call();
}
pub fn other_call(&mut self) {}
}
I just used it to make it clear that you get back to working with &mut Self and can forget that the guard is in scope.
1 Like
I'm struggling to understand what the request is here. Several of the posts here make it sound like all you're after is "run arbitrary code on drop" which is pretty much identical in Rust and in C++.
It seems clear that the code you want to run on drop is doing something to some other object, but it's not clear what exactly. If the field you want to "sanitize" is public, then this is trivial (assuming "sanitization" is a solved problem, but I have no idea what you mean by that). But if it's not public, then it would be a gaping hole in the privacy system if it was possible in safe code. And those are both true in C++ as well. So... what's the issue exactly?
2 Likes
The floating closure replacement isn't really acceptable to me, as a good solution. I get it will work. I don't want users have to go find these things. That's the big downside of using something generic like this. What you are reading tells you nothing about what it's doing when you read it, where my original example is very explicit as to what is going on and it doesn't require finding unattached global functions to pass to it and such.
And presumably the reset method has to work with a Foo, whereas mine can work with anything? I'm sure some extra layer could be done to get around that, but still, all these options (though I appreciate the effort you guys are making) are not up to snuff, IMO.
I'm struggling to understand what the request is here. Several of the posts here make it sound like all you're after is "run arbitrary code on drop" which is pretty much identical in Rust and in C++.
I really don't want to type all that again. I'd just be typing what I did in my first post. It doesn't require access to private methods. And it's not trivial because of ownership issues, which is why these guys are going through all of these hoops to try to come up with something that works, which are still kind of klunky IMO.
[I can't post anymore since this is my first day, so I can't respond I guess until tomorrow]
You've not made it clear.
So, if I understand the implicit request, which you did not say at any point, you want to be able to do
struct Foo {
field: Field,
// other fields
}
impl Foo {
pub fn other_stuff(&mut self) {}
pub fn method(&mut self) {
self.other_stuff();
{
let _janitor = SpecializedJanitor(&mut self.field);
self.other_stuff(); // <-- IMPORTANT! I can still use `self` here
}
self.other_stuff();
}
}
This is impossible in Rust because of the rules around &mut references, as you say. But with a minor rewrite of this:
struct Foo {
field: Field,
// other fields
}
impl Foo {
pub fn other_stuff(&mut self) {}
pub fn method(&mut self) {
self.other_stuff();
{
let mut janitor = SpecializedJanitor(&mut *self);
let this: &mut Self = &mut janitor;
this.other_stuff();
}
self.other_stuff();
}
// or
pub fn method(&mut self) {
self.other_stuff();
{
let mut guard = scopeguard::guard(&mut *self, |this| specialized_cleanup(&mut this.field));
let this: &mut Self = &mut guard;
this.other_stuff();
}
self.other_stuff();
}
}
this works perfectly fine, without any changes to Rust's borrowing rules.
You could make an argument that Rust could benefit from a defer statement or similar that allows taking references that aren't "activated" until the end of scope. But it's incorrect to say that what it allows isn't accomplishable today without the feature. scopeguard and its wide use are evidence to the contrary.
And I'd say finding a function is equivalent to finding a type in required cognitive effort. Knowing whether the type captures a regular reference or a special "deferred" borrow makes the special janitor types much more difficult to undertsand locally than scopeguard, which uses the same borrow rules as everything else in the language.
For the sake of completeness, here is the equivalent of the original example.
And you'll notice it requires a little more legwork. Let's go through why that's necessary.
- One of Rust's core tenets is that you a mutable reference to memory must be unique. You can't avoid this - it's how we can guarantee safety.
- So, we can't give the
BoolJana mutable reference to a field of a struct while also using one ourselves. The memory it's pointing to would be aliased.
- So, we can't give the
- So now we know that the janitor must borrow the whole struct, how can we use it ourselves? We need the janitor to promise us it won't use the memory while we are.
- Without this, we couldn't tell if the use of
BoolJanwas safe without looking at its' implementation and checking it was. There'd be nothing stopping it from starting a thread and changing the struct out from under us. - The janitor can make this promise by giving us a mutable reference. So long as we hold onto it, the janitor isn't allowed to use the struct. We request it with
janitor.hold()and hold ontothisuntil we are okay with the janitor using the struct again.
- Without this, we couldn't tell if the use of
It turns out this nested borrowing is quite a common use case in rust, and so we have the Deref* traits to make it nice to use. This eliminates the need to call hold because rust can do it implicitly whenever we need a &mut Foo. Anything less and we wouldn't have enough context to determine the safety of the function.
3 Likes
As far as I can tell, the idea behind this whole thread still being alive is that instead of the already proposed approach
let x: &mut Something = //...
// do sth
{
let guard = specialized_guard(x);
let x = guard.as_mut();
// use inner x
}
// use outer x again
which could (as mentioned) be shortened with a macro, and which unfortunately doesn’t quite work with self syntactically, there ought to be another way. The proposed approach is not “nice” enough, reason being: You need a local copy of x there which is essentially the same as the outer x, but you know it isn’t really the same x and there ought to be (and if not change the language) a way to instead write something like
let x: &mut Something = //...
// do sth
{
let guard = specialized_guard(&mut#reborrow_delayed *x);
// use outer x
}
// use outer x
Unfortunately using the same outer x is not safe. The x’s are not the same in the following regard: The inner x has a shorter life-time and this fact is significant. As demonstrated for example with this code:
use crossbeam_utils::thread;
let x: &mut Something = //...
// do sth
thread::scope(|s| {
// do sth more
{
let guard = specialized_guard(&mut#reborrow_delayed *x);
let _handle = s.spawn(|_| /* use outer x */);
// can’t use outer x here anymore
}
// can’t use outer x here anymore
}).unwrap();
// can use outer x again
Here, all the sudden, a different thread could access x mutably in parrallel with the guard being dropped. This kind of code would be rejected with a distinct, shorter-lived inner x, since the spawned scoped thread can only be passed references with a lifetime of at least the scope s.
7 Likes
Apart from my first gut reaction of "citation needed" to the strong assertion of Rust's error handling being "brutally tedious", I fail to see why it would help in this situation if error handling was a separate mechanism.
As to the problem of guarding (and mutably borrowing) a field while not restricting access to self: you can just guard and borrow the field directly, so that the compiler sees the partial borrow, and it can then allow access to the rest of the fields.
1 Like
This. Interestingly, this is exactly the kind of subtlety I was referring to when I recently wrote that piling up exemptions from borrowing rules won't do any good.
No, it doesn't.
It requires passing a Fn, or actually a FnOnce, with a particular signature. A closure is the simplest example for demonstration purposes, however free functions and associated functions also implement Fn (and thus FnOnce) and therefore are eligible.
Example: Janitor::new(value, Value::divide_by_two).
No, it doesn't.
You can create a function which creates a "common" janitor, then reuse this function all over the place.
Example:
fn janitor_divide_by_two<'a>(v: &'a mut Value)
-> Janitor<&'a mut Value, impl for<'b> Fn(&'b mut Value)>
{
Janitor::new(v, |v| v.0 /= 2)
}
It is certainly a tad more involved, and making it generic will require defining a trait for the generic operations to perform.
The alternative, if you want template-like behavior, being to use a macro.
Meta-comment:
- Please make you requirements clearer. You are familiar with the solution you use, but we are not. You gave one example from which we were apparently supposed to guess what to do, and then keep complaining that some requirements that are only obvious to you are not met. Sorry, but we do not read minds here.
- Please investigate the solutions given. You blurt out "doesn't work" without, it seems, ever actually trying to make it work. In my case, what ever made you think it would always require a closure? Did you even try to pass something else? I would guess not, since it just works.
You appear (to me) very antagonistic in this conversation, and appear (to me) dead-set on pushing for a change in the language, and I worry that you do not give the solutions you are presented with a fair evaluation -- certainly, nothing in your comments suggest that you do.
18 Likes
Of course you can move self.
If you really want to nest "janitors" you can have them take an object of type T where T: AsRef<YourType> or an ad-hoc trait if you want to "modify" the behavior, so you can pass one "janitor" to the constructor of another.
An example of this pattern is BufReader.
In general, this is a pattern that should be used relatively rarely, so there's not much of a point to attempt to "improve it".
1 Like
You appear (to me) very antagonistic in this conversation, and appear (to me) dead-set on pushing for a change in the language
I'm suffering from terminology burnout, so by 'it requires a closure' I really meant it requires passing a callback to the janitor, closure or function, it's sub-optimal compared to the C++ solution. I'm just looking for an optimal solution.
As to not explaining, I mentioned a Reddit discussion, which I thought I had linked to, but apparently forgot to. No one pointed that out so I never realized it. I didn't want to try to recreate all of that here. Actually, someone else later linked to it.
As to not trying things, there's been way too many for me to try (here and Reddit) in the sort time possible, so I can only try to figure out what you arguing by looking at your examples in the immediate term. And some of my responses were based on what I'd understood from other people saying what was and wasn't possible over on the Reddit discussion. Maybe they were wrong or there was a misunderstanding there. Part of it is that a lot of folks kept doing a completely different sort of scenario that wasn't clear to me was applicable or not.
Anyhoo, I'm not antagonistic. Everyone thinks that someone who disagrees with them is antagonistic. I'm just arguing for what I think is an optimal solution. And I have had some time this morning to play with one of the scenarios, which I'll comment on separately.
The tedious part comes from the fact that you are making changes to self that have to be undone before any return. If that's not done automatically, then suddenly you have to do it at every return point manually. That means suddenly no use of the ? return short cut and all that. Am I missing something there? I mean that's one of the fundamental reasons why those of us back in the day soundly rejected manual returns in favor of exceptions, because it's trivially easy for someone later to modify the code and forget to undo something.
On the borrowing of the just the field, now I'm confused. It's been (I think) stated by multiple people that this doesn't work, and that the only way to make it work is to borrow the whole object, which is at the root of the issue. This was much discussed above, so are they wrong?
What is the rarely part? You mean having more than one janitor applied to an object in teh same method? If so, that's not THAT rare, and if it's hacky to deal with it would be quite sub-optimal.
So I played with matthieum's solution above. One thing that doesn't work well that I may be missing something about is that the point of the janitor is not to set the value back to an arbitrary value (though that's one option that could also be supported), it's to return it back to its original value.
But I'm kind of iffy on how a callback scenario would do that. I don't think the callback can have access to the janitor, since it doesn't exist yet (it's being created), so it can get the current value but not store in the janitor. And the janitor itself knows nothing of the target struct because it depends on the callback to do all of that interaction.
Am I missing something there?
A technical tip on using discourse.
collapsed since it's rather off-topic ^^
You can select text someone wrote here and a "Quote" button will appear that allows you to "properly" create a quotation (with username and link to the original comment and a button to expand it and everything) (if you are currently already writing a reply, it will be inserted, otherwise the editor will open for a new repply). You could furthermore, if you're still limited in your activity here (due to being new), combine answers to multiple people into one reply by inserting quotes instead of using the reply button and also tagging them (with @ and the username) to make sure they're still notified.
1 Like
I think I'm out of the penalty box now.
Ah, so what you want is something like
// does not compile, just example
let mut is_danger_section = false;
assert_eq!(is_danger_section, false);
{
let janitor = for_this_scope_set(&mut is_danger_section, true);
assert_eq!(is_danger_section, true);
}
assert_eq!(is_danger_section, false);
Done using scopeguard and no custom types, it'd look something like this:
use scopeguard; // 1.1.0
#[derive(Debug, Default)]
struct HeavyStruct {
is_critical_section: bool,
// other fields
}
fn main() {
let mut resource = HeavyStruct::default();
dbg!(&resource);
{
let mut wrapper = {
let temp = resource.is_critical_section;
scopeguard::guard(&mut resource, move |this| this.is_critical_section = temp)
};
wrapper.is_critical_section = true;
dbg!(&wrapper);
}
dbg!(&resource);
}
And with a bit of generics magic, you can provide a typed abstraction around it:
Implementation details
pub struct TemporarilySet<'a, Type, Field: 'a, Accessor>
where
Accessor: for<'b> FnMut(&'b mut Type) -> &mut Field,
{
wrapped: &'a mut Type,
accessor: Accessor,
reset_to: Field,
}
impl<'a, Type, Field: 'a, Accessor> std::fmt::Debug for TemporarilySet<'a, Type, Field, Accessor>
where
Type: std::fmt::Debug,
Field: std::fmt::Debug,
Accessor: for<'b> FnMut(&'b mut Type) -> &mut Field,
{
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("TemporarilySet")
.field("wrapped", &self.wrapped)
.field("reset_to", &self.reset_to)
.field("accessor", &format_args!("{{closure}}"))
.finish()
}
}
impl<'a, Type, Field: 'a, Accessor> TemporarilySet<'a, Type, Field, Accessor>
where
Accessor: for<'b> FnMut(&'b mut Type) -> &mut Field,
{
pub fn new(wrapped: &'a mut Type, mut value: Field, mut accessor: Accessor) -> Self {
std::mem::swap(accessor(wrapped), &mut value);
TemporarilySet {
wrapped,
accessor,
reset_to: value,
}
}
}
impl<'a, Type, Field: 'a, Accessor> std::ops::Deref for TemporarilySet<'a, Type, Field, Accessor>
where
Accessor: for<'b> FnMut(&'b mut Type) -> &mut Field,
{
type Target = Type;
fn deref(&self) -> &Type {
&*self.wrapped
}
}
impl<'a, Type, Field: 'a, Accessor> std::ops::DerefMut for TemporarilySet<'a, Type, Field, Accessor>
where
Accessor: for<'b> FnMut(&'b mut Type) -> &mut Field,
{
fn deref_mut(&mut self) -> &mut Type {
&mut *self.wrapped
}
}
impl<'a, Type, Field: 'a, Accessor> Drop for TemporarilySet<'a, Type, Field, Accessor>
where
Accessor: for<'b> FnMut(&'b mut Type) -> &mut Field,
{
fn drop(&mut self) {
std::mem::swap((self.accessor)(self.wrapped), &mut self.reset_to);
}
}
#[macro_export]
macro_rules! temporarily_set {
($wrapped:ident, $path:expr, $value:expr) => {
$crate::TemporarilySet::new(&mut $wrapped, $value, |$wrapped| &mut $path)
};
}
#[derive(Debug, Default)]
struct HeavyStruct {
is_critical_section: bool,
// other fields
}
fn main() {
let mut resource = HeavyStruct::default();
dbg!(&resource);
{
let wrapper = temporarily_set!(resource, resource.is_critical_section, true);
dbg!(&wrapper);
}
dbg!(&resource);
}
With the desired output:
[src/main.rs:82] &resource = HeavyStruct {
is_critical_section: false,
}
[src/main.rs:85] &wrapper = TemporarilySet {
wrapped: HeavyStruct {
is_critical_section: true,
},
reset_to: false,
accessor: {closure},
}
[src/main.rs:87] &resource = HeavyStruct {
is_critical_section: false,
}
2 Likes