Lint against single use lifetimes

[faern]

Removes superfluous lifetime definitions. Simplifying the code. Here is the docs for this lint: https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html#single-use-lifetimes

Since we obviously had a pretty large number of these, I think the lint can help us keep code simpler. The only potential red flag I see is the lint docs saying "Also, there are some known false positives". I did not stumble over any false positives (that I noticed) when doing this small refactoring. If it turns out to be too troublesome we can disable the lint again. Or add #[allow(single_use_lifetimes)] to the places where it does not work as intended.

---

This change is 

[MarkusPettersson98]

Reviewed 13 of 13 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[faern]

Reviewable status: 12 of 13 files reviewed, all discussions resolved (waiting on @MarkusPettersson98)

---

talpid-core/src/dns/windows/tcpip.rs line 113 at r1 (raw file):

    guid: &str,
    service: &str,
    nameservers: impl Iterator<Item = &IpAddr>,

This is probably a false positive of this lint. I could not solve it. Since impl Trait requires named lifetimes, but that lifetime becomes a single use lifetime, and the lint will trigger.. But since IpAddr is a Copy type, the more idiomatic thing to do would be to iterate over them by value. So I decided to solve the problem like this instead.

If we in the future hit the same issue with a non-copy type we can just ignore the lint on a case by case basis with #[allow(single_use_lifetimes)]

[MarkusPettersson98]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

---

talpid-core/src/dns/windows/tcpip.rs line 113 at r1 (raw file):

Previously, faern (Linus Färnstrand) wrote…

This is probably a false positive of this lint. I could not solve it. Since impl Trait requires named lifetimes, but that lifetime becomes a single use lifetime, and the lint will trigger.. But since IpAddr is a Copy type, the more idiomatic thing to do would be to iterate over them by value. So I decided to solve the problem like this instead.

If we in the future hit the same issue with a non-copy type we can just ignore the lint on a case by case basis with #[allow(single_use_lifetimes)]

I think this is a reasonable take

[Serock3]

The changes made to the few places where the lint triggered seem to make the code more readable, so I am in favor

[Serock3]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @faern)

---

talpid-core/src/dns/windows/tcpip.rs line 113 at r1 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

I think this is a reasonable take

I agree

[faern]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Serock3)

---

talpid-core/src/dns/windows/tcpip.rs line 96 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Super tiny nit:

        servers.iter().filter(|addr| addr.is_ipv4()).copied(),

Why is this better? I suppose you mean because then we don't need to copy the items that don't match the filter? But these types are super tiny, and the program need to read their memory anyway to compute is_ipv4, so I'm sure the iterator chain will dereference the memory of all items not matching the filter anyway.

Copy in general is usually for small simple types, so copying them around is usually not a noticeable cost. I would very much understand the reasoning if it was .cloned() we were talking about.

I can change the code, because I don't have a strong opinion. I just want to know what your rationale is here.

[Serock3]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @faern)

---

talpid-core/src/dns/windows/tcpip.rs line 96 at r2 (raw file):

Previously, faern (Linus Färnstrand) wrote…

Why is this better? I suppose you mean because then we don't need to copy the items that don't match the filter? But these types are super tiny, and the program need to read their memory anyway to compute is_ipv4, so I'm sure the iterator chain will dereference the memory of all items not matching the filter anyway.

Copy in general is usually for small simple types, so copying them around is usually not a noticeable cost. I would very much understand the reasoning if it was .cloned() we were talking about.

I can change the code, because I don't have a strong opinion. I just want to know what your rationale is here.

My rationale is that I thought it said.cloned() 🙂, if it's a Copy type then it doesn't really matter. Putting the filter first looks a bit more natural to me but that's just taste I guess.

[Serock3]

Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved

[Serock3]

Reviewed 12 of 13 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved