Conversation
context: rust-lang/rust#98835
| .iter() | ||
| .filter(move |c| c.parent == Some(self.contents.id)) | ||
| .map(move |c| Self { | ||
| .map(move |c| CidrTree { |
There was a problem hiding this comment.
The reason for this change is that Self (which is just a sugar for CidrTree<'a>) brings the the lifetime 'a to the scope of the returned opaque type. Use CidrTree instead to allow rustc to infer a shorter lifetime (not necessarily 'a).
Generally, it's recommended to use the struct name instead of Self to allow rust to infer the appropriate lifetime rather than unnecessarily forcing a specific lifetime on the created opaque type.
The error message maybe helpful: https://crater-reports.s3.amazonaws.com/pr-98835/try%23cb1f7c96119295882e08b09b4bd01051669c071b/gh/tonarino.innernet/log.txt
There was a problem hiding this comment.
Oh this is an interesting one! I've often used Self in places like this, good to know the kind of issues that can cause.
strohel
left a comment
There was a problem hiding this comment.
This is an interesting one! It also didn't occur to me that saying Self could bring an unwanted lifetime bound in. Approving.
@aliemjay can you please rebase on main before we merge this? Or I can do that if I have force-push permissions to your branch. We've fixed clippy warnings in the mean time, so that should make the CI green.
|
I'm on mobile now, so I can only use github ui to merge the branch, if this is not suitable, feel free to close this and commit the change yourself. |
|
@aliemjay oh that works, too. Merging this will rebase the merge away. Thanks! |
Hi,
rust-lang/rust#98835 fixes a rustc bug that makes it incorrectly accepts the following code:
The crater run found that
innernetwill break with this change because it indirectly relies on this bug. I'm not sure when this change reaches stable, if ever, but it's better to not rely on such a bug. This PR tries to fix this.If you have an opinion on this change, please let me know here or by commenting on the PR.
You can install and test the toolchain locally by running the command:
rustup-toolchain-install-master cb1f7c96119295882e08b09b4bd01051669c071b