TryWriteable should have try_writeable_borrow

[sffc]

#6985 (comment)

[robertbastian]

No I don't think so. The borrowing method should be constant time, because otherwise write_to_string would duplicate work. I don't think a scenario where an error can be detected in constant time needs to be a TryWriteable in the first place, so there's no point in making the borrowing fallible.

[sffc]

ok, maybe it should return (Option<&str>, Option<&str>) then?

Or maybe TryWriteable just shouldn't have this function?

[robertbastian]

what would that pair mean?

[sffc]

Just the result of running writeable_borrow on both the Ok and the Err variants. If either needs to be computed, then it is None. For example, often TryWriteable is implement on Result<W, &'static str>, and in that case, try_writeable_borrow might return (None, Some(<the err string>)). Haven't fully thought it through.

[robertbastian]

How would try_write_to_string then know which variant to choose?

This method is required to make TryWriteable borrow in no-alloc mode, so I'd rather not remove it.

[sffc]

What if we had a signature like

trait NewWriteSink<'a>: fmt::Write {
    fn write_finish(s: &'a str) -> fmt::Result;
}

trait TryWriteable {
    // ...
    fn try_write_bikeshed<'a, NewWriteSink>(&'a self, sink: NewWriteSink<'a>) -> Result<Result<(), Self::Error>, fmt::Error>;
}

// No-alloc impl:
struct NoAllocNewWriteSink<'a>(Option<&'a str>);
impl fmt::Write for NoAllocNewWriteSink<'_> {
    // functions that set the inner option to None and return an error
}
impl<'a> NewWriteSink<'a> for NoAllocNewWriteSink<'a> {
    fn write_finish(s: &'a str) -> fmt::Result {
        self.0 = Some(s);
        Ok(())
    }
}

[sffc]

• @robertbastian We could just remove it in 2.1; we don't seem to use it
• @robertbastian We could use it in impl TryWriteable for PotentialUtf8
• @sffc That's a linear-time impl; is that ok?
• @robertbastian Yeah it could be ok
• @sffc I think we should be able to borrow in either the Ok or the Err case
• @sffc I think the correct signature is Option<Result<&str, &str>>
• @robertbastian Maybe, but it still needs the error code in the error case. So it would be Option<Result<&str, (Self::Error, &str)>>. Let's just wait until we really need it

[Manishearth]

This can be removed from the milestone or closed, yes?