Skip to content

Borrow checker bug: cannot return reference to temporary value #66541

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

Open
andreytkachenko opened this issue Nov 19, 2019 · 10 comments
Open

Borrow checker bug: cannot return reference to temporary value #66541

andreytkachenko opened this issue Nov 19, 2019 · 10 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@andreytkachenko
Copy link

Hi, here is an example of very strange error I have stuck with:

struct Baz {
    pub foo: &'static str
}

trait Foo {
    fn foo() -> &'static str;
}

trait Bar {
    fn bar() -> &'static [Baz];
}

struct Test;

impl Foo for Test {
    fn foo() -> &'static str {
        "Hello, World!"
    }
}

struct Bat {
    test: Test
}

impl Bar for Bat {
    fn bar() -> &'static [Baz] {
        use Foo;

        &[
            /// This compiles 
            {
                Baz { foo: "Hello, World!" }
            },
            
            /// but, this cause an error "cannot return reference to temporary value"
            {
                let foo = "Hello, World!";
                Baz { foo }
            },
            
            /// this cause an error, also
            {
                let foo = Test::foo();
                Baz { foo }
            },
            
            /// and this cause an error, too
            {
                Baz { foo: Test::foo() }
            }
        ]
    }
}

and the error message:

error[E0515]: cannot return reference to temporary value
  --> src/lib.rs:29:9
   |
29 |            &[
   |  __________^-
   | | _________|
   | ||
30 | ||             /// This compiles 
31 | ||             {
32 | ||                 Baz { foo: "Hello, World!" }
...  ||
50 | ||             }
51 | ||         ]
   | ||         ^
   | ||_________|
   | |__________returns a reference to data owned by the current function
   |            temporary value created here
@Centril Centril added A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 19, 2019
@Centril
Copy link
Contributor

Centril commented Nov 19, 2019

cc @matthewjasper @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Nov 19, 2019

The other 3 examples not compiling is not surprising, they contain runtime code, so any reference to it will return a reference to a local variable.

I think the suprising thing to folks is that the first example compiles. I have no idea how to improve diagnostics to the point where this difference becomes clear unless we introduce const {} blocks and add a lint to force all implicit promoteds to become explicit.

@jonas-schievink jonas-schievink added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Nov 19, 2019
@csmoe
Copy link
Member

csmoe commented Nov 19, 2019

@oli-obk https://doc.rust-lang.org/reference/expressions.html#temporary-lifetimes this chapter in reference may help for an explanation?

@andreytkachenko
Copy link
Author

Yes, thanks for this doc links, something become more clear, but it is still not obvious with this case:

     ....
     Baz { foo: Test::foo() }
     .....

@andreytkachenko
Copy link
Author

Why this not compiles?

const fn foo() -> &'static str {
    "Hello, World!"
}

fn test() -> &'static [&'static str] {
    &[
        "Line 1",
        foo(),
    ]
}

@oli-obk
Copy link
Contributor

oli-obk commented Nov 19, 2019

Before getting into that, note that

const fn foo() -> &'static str {
    "Hello, World!"
}

const FOO: &'static str = foo();

fn test() -> &'static [&'static str] {
    &[
        "Line 1",
        FOO,
    ]
}

does in fact compile.

The underlying reason is that const fn can do things like panicking. While const items can also panic, the difference is that if the user writes &foo() right now and foo panics, they get a runtime panic. If we automatically uplifted const fn calls to get promoted, the users perfectly compiling code would now fail to compile. Considering that the foo() call may be behind other checks, have arguments or generic parameters, this is a totally valid use case that we'd suddenly stop supporting.

@andreytkachenko
Copy link
Author

Ok. I still has a question. Is this how the compiler intended to work by RFC or it is just current limitations of implementation (meaning unable to use a result of a const function for static slice)? And one more, is this issue has something what may be considered as an issue or should I just close it, since everyone agreed that it is "working as expected"?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 20, 2019

This is not a limitation of the implementation. While the original promoted locals RFC suggested to allow const fn calls, it has since been discovered that doing so poses forward compatibility problems that we'd rather avoid. Since you can always insert an intermediate constant, this seems easily worked around. What we would like to see is a const { foo() } block that allows you to create anonymous constants anywhere, thus working around the compatibility worries, because the user explicitly opted in to const evaluation. Such a feature will need an RFC though (although it should not be a very complex one, as the feature itself is likely very simple. There are only questions around the ability to use generics of the surrounding function that may be controversial)

@JakkuSakura
Copy link

I had this simplest example for this "bug". Now I understand why it is intended. It would be nice to have const {} or infer const property from source code.

trait AsStr {
    fn as_str(&self) -> &str;
}

impl AsStr for &[u8] {
    fn as_str(&self) -> &str {
        unsafe { str::from_utf8_unchecked(self) }
    }
}

fn foo(buf: &[u8]) -> &str {
    unsafe { str::from_utf8_unchecked(buf) }
}

// Error: cannot return value referencing temporary value
fn bar(buf: &[u8]) -> &str {
    return buf.borrow().as_str();
}

// Okay
fn baz(buf: &[u8]) -> &str {
    return foo(buf);
}

// Error: cannot return value referencing function parameter `buf`
fn qux(buf: &[u8]) -> &str {
    return AsStr::as_str(&buf);
}

@oli-obk
Copy link
Contributor

oli-obk commented Aug 26, 2020

@qiujiangkun I don't think your example relates to this issue. You are just getting a borrow check error. The examples here are actually fully constant and don't rely on arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants