Meaning of validity after a mutator panic

[HeroicKatora]

The protecting code of mutators does not protect the (implicit) panic exit point with assertions of the invariant holding. This makes it possible to observe an instace of a tightness-defined type whose invariant is in fact violated.

bound!(Letter: char where |c| c.is_alphabetic());

struct Observer(Letter);

impl Drop for Observer {
    fn drop(&mut self) {
        if !self.0.is_alphabetic() {
            println!("Woah there");
        }
    }
}

let mut obs = Observer(Letter::new('a').unwrap());
obs.0.mutate(|l| *l = '0'); // Not, in fact, alphabetic

// Prints: Woah there

However, for usual validity invariants such as those upheld by str or NonZeroU8 etc. even this kind of observation is forbidden. This may be a concious design decision as the only 'correct' way to handle this would be an immediate program abort instead of a usual panic. For the fallback mutate_or it is at least theoretically possible to handle it correctly while continuing to panic but you need to assign to self through a drop-guard instead of a code path that is never reached on panic unwinding.

[PabloMansanet]

Thanks! that's a very interesting point.

I think some update to the documentation is in order, because the current Bound type is weaker by design than what the documentation may hint at. For one, it hinges on the invariant function being pure (it's trivial to create invalid Bound types by just having a RNG call or some external state referenced in the invariant check).

At the moment, the only hard "promises" are that the invariant is asserted after construction and on mutation exit.

That said, I'm working on a StronglyBound variation that requires an unsafe StrongBound trait implementor (to ask the user to ensure manually it's pure), which then makes liberal use of unsafe_unchecked to promise the compiler the inner type always upholds the invariant. For that one, I'm probably going to go the way you suggest, with an abort.

[HeroicKatora]

Note that it does not hinge on the validator alone. Indeed, we can panic exit within the mutator to trigger the same issue:

// Setup code like above

let mut obs = Observer(Letter::new('a').unwrap());
obs.0.mutate(|l| {
    *l = '0';
    panic!("Avoid ever checking the validator")
});

[PabloMansanet]

Seems then like this is mainly a problem with the mutate method. I think there are ways that try_mutate, mutate_into and mutate_or can be refactored not to cause this issue.

I will probably leave mutate for the "weak" bound option (with more clarification in the docs), because as long as we don't trigger undefined behaviour, I feel like it's just a matter of expectations management, e.g. convey to the user that they must avoid panicking during mutation. If they don't comply with this, it will be a logic bug, but not undefined behaviour.

The mutate method, as is, will have to be out for the future StrongBound variant, for the reasons you've brought up (or at the very least marked unsafe).