Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow pass by reference if we return a reference #2951

Merged
merged 4 commits into from
Jul 24, 2018
Merged

Allow pass by reference if we return a reference #2951

merged 4 commits into from
Jul 24, 2018

Conversation

etaoins
Copy link
Contributor

@etaoins etaoins commented Jul 23, 2018

Currently this code will trigger trivally_copy_pass_by_ref:

struct OuterStruct {
    field: [u8; 8],
}

fn return_inner(outer: &OuterStruct) -> &[u8] {
    &outer.field
}

If we change the outer to be pass-by-value it will not live long enough for us to return the reference. The above example is trivial but I've hit this in real code that either returns a reference to either the argument or in to self.

This suppresses the trivally_copy_pass_by_ref lint if we return a reference and it has the same lifetime as the argument. This will likely miss complex cases with multiple lifetimes bounded by each other but it should cover the majority of cases with little effort.

Fixes #2946

Currently this code will trigger `trivally_copy_pass_by_ref`:

```
struct OuterStruct {
    field: [u8; 8],
}

fn return_inner(outer: &OuterStruct) -> &[u8] {
    &outer.field
}
```

If we change the `outer` to be pass-by-value it will not live long
enough for us to return the reference. The above example is trivial but
I've hit this in real code that either returns a reference to either the
argument or in to `self`.

This suppresses the `trivally_copy_pass_by_ref` lint if we return a
reference and it has the same lifetime as the argument. This will likely
miss complex cases with multiple lifetimes bounded by each other but it
should cover the majority of cases with little effort.
@flip1995
Copy link
Member

Yeah this won't catch

fn good_return_explicit_lt_ref_bound<'a, 'b: 'a>(foo: &'b Foo) -> &'a u32 {
    &foo.0
}

I think we need to check somehow if the lifetime of the argument outlives the lifetime of the return value. But I don't know how to do this off the top of my head.

It would be great if you could add this to the Known Problems section of the lint. Everything else LGTM.

@etaoins
Copy link
Contributor Author

etaoins commented Jul 23, 2018

@flip1995 Updated with a note in the Known Problems section

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants