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

borrowck treatment of struct-tuple more liberal than struct with fields #10902

Closed
pnkfelix opened this issue Dec 10, 2013 · 3 comments · Fixed by #17199
Closed

borrowck treatment of struct-tuple more liberal than struct with fields #10902

pnkfelix opened this issue Dec 10, 2013 · 3 comments · Fixed by #17199
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@pnkfelix
Copy link
Member

Forked off of #10766

A question in #rust regarding a borrow checker error led me on the following exploration.

The four variants are labelled one, tup, two_tuple, and two_fields

  • one is just repeating the test case from the description (of Should &'a Value as &'a Trait work? #10766). Its a sanity check that the old error message and behavior still persists.
  • two_fields is closest to what achin described in his bug report. The problem that the borrow checker reports for that code is interesting, and it is replicated by the more minimal test case at the end of this comment.
  • The other two cases are variants of two_fields that I made while trying to make as minimal a test case as possible.
% rustc --cfg two_fields --cfg two_tup /tmp/achin.rs
/tmp/achin.rs:32:8: 32:9 error: cannot infer an appropriate lifetime for region in type/impl due to conflicting requirements
/tmp/achin.rs:32         P{ car: v1 as &'a T, cdr: v2 as &'a T }
                         ^
/tmp/achin.rs:32:34: 32:45 note: first, the lifetime must be contained by the expression at 32:34...
/tmp/achin.rs:32         P{ car: v1 as &'a T, cdr: v2 as &'a T }
                                                   ^~~~~~~~~~~
/tmp/achin.rs:32:34: 32:45 note: ...so that automatically borrowed pointer is valid at the time of borrow
/tmp/achin.rs:32         P{ car: v1 as &'a T, cdr: v2 as &'a T }
                                                   ^~~~~~~~~~~
/tmp/achin.rs:32:16: 32:27 note: but, the lifetime must also be contained by the expression at 32:16...
/tmp/achin.rs:32         P{ car: v1 as &'a T, cdr: v2 as &'a T }
                                 ^~~~~~~~~~~
/tmp/achin.rs:32:16: 32:27 note: ...so that automatically borrowed pointer is valid at the time of borrow
/tmp/achin.rs:32         P{ car: v1 as &'a T, cdr: v2 as &'a T }
                                 ^~~~~~~~~~~
error: aborting due to previous error
task 'rustc' failed at 'explicit failure', /Users/fklock/Dev/Mozilla/rust.git/src/libsyntax/diagnostic.rs:102
task '<main>' failed at 'explicit failure', /Users/fklock/Dev/Mozilla/rust.git/src/librustc/lib.rs:396

The interesting thing is that two_tuple (and likewise tup) do not exhibit that error, even though they are completely analogous code. This is probably representative of a bug somewhere in the borrow checker since it really should be treating a struct-tuple here the same as a struct with named fields. (Plus, you know, these borrows look okay to me... but I have not yet dived in and tried to understand what the problem is that they are trying to flag.)

The code:

#[cfg(one)]
mod one {
    trait T {}

    fn f<'a, V: T>(v: &'a V) -> &'a T {
        v as &'a T
    }
}

#[cfg(tup)]
mod tup {
    trait T {}
    fn f<'a, V: T>(v1: &'a V, v2: &'a V) -> (&'a T, &'a T) {
        (v1 as &'a T, v2 as &'a T)
    }
}

#[cfg(two_tuple)]
mod two {
    trait T {}
    struct P<'a>(&'a T, &'a T);
    fn f<'a, V: T>(mut v1: &'a V, v2: &'a V) -> P<'a> {
        P(v1 as &'a T, v2 as &'a T)
    }
}

#[cfg(two_fields)]
mod two {
    trait T {}
    struct P<'a> { car: &'a T, cdr: &'a T }
    fn f<'a, V: T>(mut v1: &'a V, v2: &'a V) -> P<'a> {
        P{ car: v1 as &'a T, cdr: v2 as &'a T }
    }
}

This is a slightly reduced test from the original one above (that was originally a comment on #10766).

#[cfg(two_tuple)]
pub mod two {
    trait T {}
    struct P<'a>(&'a T, &'a T);
    pub fn f<'a>(car: &'a T, cdr: &'a T) -> P<'a> {
        P(car, cdr)
    }
}

#[cfg(two_fields)]
pub mod two {
    trait T {}
    struct P<'a> { car: &'a T, cdr: &'a T }
    pub fn f<'a>(car: &'a T, cdr: &'a T) -> P<'a> {
        P{ car: car, cdr: cdr }
    }
}

fn main() {}

And here are the results:

% rustc --version
/Users/fklock/opt/rust-dbg/bin/rustc 0.9-pre (f817ed3 2013-12-09 13:51:32 -0800)
host: x86_64-apple-darwin
% rustc --cfg two_tuple /tmp/achin.rs 
% rustc --cfg two_fields /tmp/achin.rs 
/tmp/achin.rs:15:8: 15:9 error: cannot infer an appropriate lifetime for region in type/impl due to conflicting requirements
/tmp/achin.rs:15         P{ car: car, cdr: cdr }
                         ^
/tmp/achin.rs:15:16: 15:19 note: first, the lifetime must be contained by the expression at 15:16...
/tmp/achin.rs:15         P{ car: car, cdr: cdr }
                                 ^~~
/tmp/achin.rs:15:16: 15:19 note: ...so that automatically borrowed pointer is valid at the time of borrow
/tmp/achin.rs:15         P{ car: car, cdr: cdr }
                                 ^~~
/tmp/achin.rs:15:26: 15:29 note: but, the lifetime must also be contained by the expression at 15:26...
/tmp/achin.rs:15         P{ car: car, cdr: cdr }
                                           ^~~
/tmp/achin.rs:15:26: 15:29 note: ...so that automatically borrowed pointer is valid at the time of borrow
/tmp/achin.rs:15         P{ car: car, cdr: cdr }
                                           ^~~
error: aborting due to previous error
task 'rustc' failed at 'explicit failure', /Users/fklock/Dev/Mozilla/rust.git/src/libsyntax/diagnostic.rs:102
task '<main>' failed at 'explicit failure', /Users/fklock/Dev/Mozilla/rust.git/src/librustc/lib.rs:393
[ERROR#1] % 
@pnkfelix
Copy link
Member Author

I wonder if this is an artifact of our need to use the type-checker to resolve the ambiguity as to whether P(e1, e2) is a tuple-construction or a function call. I.e. we need to resolve the type of P to disambiguate that case, and the RUST_LOG output I see includes that as a difference between the two cases here.

(Or it could be a red-herring. Seems more likely that its a red herring, since I do not see what it would mean to attempt to "more eagerly resolve P" in the expression P { car: e1, cdr; e2 })

@nikomatsakis
Copy link
Contributor

cc me

@treeman
Copy link
Contributor

treeman commented Sep 2, 2014

Code snippets now compiles.

$ rustc -v
rustc 0.12.0-pre (f6a7ab40e 2014-08-29 08:21:26 +0000)

Can close.

bors added a commit that referenced this issue Sep 15, 2014
…hton

Closes #7813.
Closes #10902.
Closes #11374.
Closes #11714.
Closes #12920.
Closes #13202.
Closes #13624.
Closes #14039.
Closes #15730.
Closes #15783.
flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 31, 2023
New lint [`needless_return_with_try`]

Closes rust-lang#10902

Rather than having a config option, this will just suggest removing the "return"; if `try_err` is used as well, then it'll be added again but without the `?`.

changelog: New lint [`needless_return_with_try`]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants