feat: add forget method to semaphore guards

[hlbarber]

No description provided.

[notgull]

What advantage does this have over just calling mem::forget on the locks?

[hlbarber]

If we don't provide an explicit forget method and want to provide this functionality through documentation, "prevent this permit from returning to the semaphore by calling mem::forget", we are less able to evolve the internals. It would forbid changes which would break this behaviour.

[hlbarber]

In fact, I think the forget on the owned permit is incorrect here - we want the Arc destructor to run.

[notgull]

I get the idea, although it seems somewhat useless without a manual unlock method. Although we can save that for a future PR.

[notgull]

I'd rather avoid the unsafe here. I would prefer to make the type a wrapper around Option<Arc<...>>, and then make this just take the Arc out without any of the other dropping.

[Jules-Bertholet]

1. Arc has only one niche currently, it would be a shame to use it up.
2. There is already another unsafe block in this library that performs the same operation. (Hopefully, my RFC 3466 will be accepted someday and then neither of these will be necessary…)

[notgull]

For both cases, I value safety over precedent here. smol-rs doesn't have the resources to conduct thorough unsafe audits (outside of off-the-shelf tools like Miri), so the less unsafe code we have the better.

[Jules-Bertholet]

For both cases

What do you mean by "both cases" here? Are you saying I should remove the unsafe in the RwLock impl also? (The performance cost would be much greater over there compared to here.)

[notgull]

Not in this PR, but I am skeptical of premature optimizations like this.

[Jules-Bertholet]

I've removed the unsafe.

[Jules-Bertholet]

(The performance cost would be much greater over there compared to here.)

Specifically, it would mean a branch on every access to the value behind the lock guard.

[notgull]

We can benchmark this in the future

[notgull]

Thanks!