Consider to refactor json::Encoder::buffer_encode

[edwardw]

Here is the implementation of json::Encoder::buffer_encode:

pub struct Encoder<'a> {
    wr: &'a mut io::Writer,
}
impl<'a> Encoder<'a> {
    pub fn buffer_encode<T:Encodable<Encoder<'a>, io::IoError>>(to_encode_object: &T) -> Vec<u8>  {
        let mut m = MemWriter::new();
        {
            let mut encoder = Encoder::new(&mut m as &mut io::Writer);
            let _ = to_encode_object.encode(&mut encoder);
        }
        m.unwrap()
    }
}

Note that to_encode_object has type Encodable<Encoder<'a>> so to_encode_object.encode(...) requires an Encoder instance with lifetime 'a, which in turn requires a Writer instance of lifetime 'a, but the given one m is only stack local. rustc should reject it.

The reason why this is legit now is because of a bug in variance.rs#L744-L748, where an extra contravariant constraint should be added in presence of a trait reference. That bug is a soundness issue as well as a backward compatibility hazard, manifested in bugs such as #12470 and #14285.

But the patch for the trait reference variance bug is blocked by otherwise invalid json::Encoder::buffer_encode. We need to refactor it first. Nominate.

[rustonaut]

Hy,
I am new to rust so correct me if I am wrong but doesn't to_encode_object.encode(...) require ::Encode<E> (not Encode<'a>) where E normally corresponds to IoError? And wouldn't that mean that only a reference to a struct-with-lifetime is passed as object to a function not knowing about this lifetime (and not using lifetime variables itself). Therefor it should be impossible for to_encode_object.encode(...) to generate a reference (or similar) witch can outlive the lifetime of the to_encode_object.encode(...) function. If that is the case buffer_encode would be implemented perfectly safe nonindependent of the actual implementation of encode.

As result fixing the bug mentioned in context of variance.rs#L744-L748 should not affect this code.

For reference here is the codesnipped defining the Encodable trait:

//libserialize/serialize.rs
pub trait Encodable<S:Encoder<E>, E> {
    fn encode(&self, s: &mut S) -> Result<(), E>;
}

[nrc]

I'm 'fixing' this by sticking in some transmutes on my branch, that means we can fix the underlying lifetimes bug (which I have also done) without refactoring as suggested here. Of course the problem still exists, so we still need to refactor. I have left FIXMEs in the code.

[nikomatsakis]

cc me -- I've been working (really, really slowly) on a branch with some related fixes to that area of the type system.

[barosl]

I mistakenly referenced this issue, sorry.

[steveklabnik]

Encode and friends have been moved out of tree now.