Specify the lifetime on RopeSlice::get_chars_at() and similar functions as it is specified for get_chars()

[BoydstonNicholas]

Both the get_chars() and get_chars_at() methods on RopeSlice<'a> return a value of type Chars, however, the get_chars() version returns a Chars<'a> with a lifetime matching the RopeSlice<'a>, while get_chars_at()'s return value uses an anonymous lifetime, Chars<'_>, which is limited to the scope in which the method was called.

I encountered this while trying to write a previous-word implementation, under some constraints. I found myself needing to write a function that took a slice and the cursor position, and returned an iterator over the characters in the slice traveling towards the start of the slice. I ended up with some functions that looked a bit like the following:

    fn iter_slice_rev<'a>(text: RopeSlice<'a>) -> Chars<'a> {
        let mut it = text.chars_at(text.len_chars());
        it.reverse();
        it
    }

    fn iter_slice_forward<'a>(text: RopeSlice<'a>) -> Chars<'a> {
        let it = text.chars();
        it
    }

The slice_forward() function will compile, while the slice_rev() function has borrow checker errors because the Chars instance it returns has its anonymous lifetime set to be the same lifetime as the borrow in the function which starts when chars_at() is called, and thus cannot outlive the function.

Similar function attempts also fail with lifetime errors:

    fn iter_slice_rev2<'a, 'b>(text: RopeSlice<'a>) -> Chars<'b> {
        let mut it = text.chars_at(text.len_chars());
        it.reverse();
        it
    }

    fn iter_slice_rev3<'a, 'b>(text: &RopeSlice<'a>) -> Chars<'b> {
        let mut it = text.chars_at(text.len_chars());
        it.reverse();
        it
    }

    fn iter_slice_rev4(text: RopeSlice<'_>) -> Chars<'_> {
        let mut it = text.chars_at(text.len_chars());
        it.reverse();
        it
    }

I attempted to see if there were any problems with simply adjusting the signature of the functions to return the more general lifetime, changing pub fn get_chars_at(&self, char_idx: usize) -> Option<Chars> to pub fn get_chars_at(&self, char_idx: usize) -> Option<Chars><'a> in slice.rs on line 1261, and making a similar change for get_chars(). It did not cause any issues with the test suite.

I do not believe that such a change would be a breaking change, since the lifetime is only being expanded.

A similar difference exists for chars_at(), bytes_at(), lines_at(), and chunks_at_line_break(), vs. chars(), bytes(), lines(), and all other chunks_at_*() methods.

The functions I have noticed that use the anonymous lifetime and have counterparts that use the slice's lifetime are:

• get_chunks_at_line_break()
• get_lines_at()
• get_chars_at()
• get_bytes_at()
• chunks_at_line_break()
• lines_at()
• chars_at()
• bytes_at()

The differences among the chunks_at_*() methods seem to indicate that the use of the anonymous lifetime for the former group is accidental and not a specific API choice. I am not entirely certain that such a guess would be correct however.

If this is indeed not intended, and it would be an acceptable change, then I would like to ask for the lifetime annotations to be added.

If it is acceptable and you would prefer it, I could put together a pull request for the change.

[cessen]

Thanks for catching this! I believe this is indeed a mistake on my part. I was relying on lifetime elision to do the right thing here, but it elides to the lifetime associated with the reference to self (the lifetime of the RopeSlice) rather than the lifetime associated with self itself (the lifetime of the Rope). The latter is definitely the intended behavior.

I'm happy to fix this myself.

Thanks again for the report!

[cessen]

I believe this is now fixed in master, from commit 27eb4aec85dccf09957e89860f44876c8c5dfbaf

If you could test it out to confirm that everything works as expected, that would be great! Then I can make a bug fix release.

(Also, if you look at the code, just fyi that I'm aware changing Self to RopeSlice<'a> doesn't do anything in this case. I just decided to do it for clarity, while I was changing the other ones anyway.)

[BoydstonNicholas]

Thanks.

Everything in my project compiles correctly now, and has the expected behavior.

[cessen]

Great! I'll make a new release within the next couple of days.

Thanks again for your help!

[cessen]

Just released v1.3.2 with the fix.