j-tai/getargs

Can't extract `&'static str` from `Opt::Long`

LoganDark opened this issue · 7 comments

I think your next implementation has unintentional lifetime elision where self is borrowed for the duration of 'a.

error[E0597]: `options` does not live long enough
   --> src/settings.rs:123:24
    |
123 |     while let Some(opt) = options.next() {
    |                           ^^^^^^^^^^^^^^
    |                           |
    |                           borrowed value does not live long enough
    |                           argument requires that `options` is borrowed for `'static`
...
144 | }
    | - `options` dropped here while still borrowed

The reason why rustc thinks options has to be borrowed for 'static is because of this inner match:

			if let getargs::Error::DoesNotRequireValue(opt) = err {
				match opt {
					Opt::Short(short) => Error::DoesNotRequireValueShort(short),
					Opt::Long(long) => Error::DoesNotRequireValueLong(long)
				}
			}

and my Error enum declaration says:

	DoesNotRequireValueLong(&'static str)

The strings being used with Options are 'static, so I would've expected this to work. Is there any reason why it shouldn't, or is this a mistake?

For now I can get around this with a transmute, but that is incredibly unsafe and this could probably be fixed by adjusting lifetimes a bit.

j-tai commented

The strings being used with Options are 'static, so I would've expected this to work. Is there any reason why it shouldn't, or is this a mistake?

They are not 'static. Long options contain a &'a str which is a reference to the arguments that you provided. Remember, getargs doesn't know which options are valid and which aren't, so it gives you whatever options it finds in the input.

In this case I would recommend making your Error enum have a lifetime, like this:

enum Error<'a> {
    DoesNotRequireValueLong(&'a str),
    ...
}

That way your error has the same lifetime as the arguments array that you passed into the parser.

If you actually pass in &'static strs (or similar) into the Options constructor, then you should get an Options<'static, S>, and you should be able to extract a &'static str from that.

j-tai commented

Actually, I realized that you would need an arguments array that lives for 'static in order to get an Options<'static, S>, even if the strings in the array are 'static. This is a bit tricky...

The issue is that Options accepts any impl AsRef<str> to parse. So it only has one lifetime to work with -- the lifetime of the array, not the lifetimes of the individual strings. It needs to call as_ref() in order to read the string and return substrings. These strings have the same lifetime as the array. Basically we need to have a second lifetime for the individual strings, which we don't have right now.

Actually, I realized that you would need an arguments array that lives for 'static in order to get an Options<'static, S>, even if the strings in the array are 'static. This is a bit tricky...

My options array does live for 'static. I leak it at the start of my program.

The issue is that Options accepts any impl AsRef<str> to parse. So it only has one lifetime to work with -- the lifetime of the array, not the lifetimes of the individual strings.

In this case the lifetime of the array is also 'static.

Basically we need to have a second lifetime for the individual strings, which we don't have right now.

I had this "showerthought" when sleeping earlier. If you're OK with a breaking pull request to fix this, I would be glad to open one. I'm not sure to what extent you're willing to accept breaking changes.

j-tai commented

My options array does live for 'static. I leak it at the start of my program.

If that's the case, then I'm not really sure where the 'a lifetime comes from.

If you're OK with a breaking pull request to fix this, I would be glad to open one.

Sounds good to me. I think this could be implemented together with #1, so we could change Options to something like:

struct Options<'a, I>
where
    I: Iterator<Item = &'a str>

then use a peekable iterator to parse the options.

If that's the case, then I'm not really sure where the 'a lifetime comes from.

My theory is that it comes from the &self borrow in next?

I think this could be implemented together with #1, so we could change Options to something like:

struct Options<'a, I>
where
    I: Iterator<Item = &'a str>

then use a peekable iterator to parse the options.

This looks like it would work great, then people could use any AsRef (via map) but also anything else (like argv).

I found the issue. Some of your functions do not annotate the 'a lifetime parameter of getargs::Result. So, Rust attempts to "un-elide" it as being the lifetime of &self.

This will be fixed in my PR.

j-tai commented

Makes sense. Thanks!