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

give_expl_lifetime_param is silently dropping other formal lifetime params #13058

Closed
pnkfelix opened this issue Mar 21, 2014 · 7 comments · Fixed by #18555
Closed

give_expl_lifetime_param is silently dropping other formal lifetime params #13058

pnkfelix opened this issue Mar 21, 2014 · 7 comments · Fixed by #18555
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

(See also #13057 for the non-deterministic behavior of give_expl_lifetime_param.)

For the input file below, the invocation of rustc that prompts with the "consider using an explicit lifetime parameter" hint is providing an erroneous signature. The reason the provided signature is erroneous is because there is a formal lifetime parameter 'r that is bound in the generics and also used in the trait bounds of the generics, but the suggested signature drops the formal 'r parameter (while adding 'a) but leaves the existing reference to 'r.

Input file (demo.rs):

use std::iter::{Range,range};

trait Itble<'r, T, I: Iterator<T>> { fn iter(&'r self) -> I; }

impl<'r> Itble<'r, uint, Range<uint>> for (uint, uint) {
    fn iter(&'r self) -> Range<uint> {
        let &(min, max) = self;
        range(min, max)
    }
}

fn check<'r, I: Iterator<uint>, T: Itble<'r, uint, I>>(cont: &T) -> bool
{
    let cont_iter = cont.iter();
    let result = cont_iter.fold(Some(0u16), |state, val| {
        state.map_or(None, |mask| {
            let bit = 1 << val;
            if mask & bit == 0 {Some(mask|bit)} else {None}
        })
    });
result.is_some()
}

fn main() {
    check((3u, 5u));
}

Transcript:

% uname -a
Darwin fklock-Oenone.local 12.5.0 Darwin Kernel Version 12.5.0: Sun Sep 29 13:33:47 PDT 2013; root:xnu-2050.48.12~1/RELEASE_X86_64 x86_64
% rustc --version
/Users/fklock/opt/rust-dbg/bin/rustc 0.10-pre (b6d5b8f 2014-03-17 00:21:59 -0700)
host: x86_64-apple-darwin


% rustc --crate-type lib /tmp/demo.rs
/tmp/demo.rs:12:1: 22:2 note: consider using an explicit lifetime parameter as shown: fn check<'a, I: Iterator<uint>, T: Itble<'r, uint, I>>(cont: &'a T) -> bool
/tmp/demo.rs:12 fn check<'r, I: Iterator<uint>, T: Itble<'r, uint, I>>(cont: &T) -> bool
/tmp/demo.rs:13 {
/tmp/demo.rs:14 let cont_iter = cont.iter();
/tmp/demo.rs:15 let result = cont_iter.fold(Some(0u16), |state, val| {
/tmp/demo.rs:16 state.map_or(None, |mask| {
/tmp/demo.rs:17 let bit = 1 << val;
...
/tmp/demo.rs:14:21: 14:32 error: cannot infer an appropriate lifetime for autoref due to conflicting requirements
/tmp/demo.rs:14 let cont_iter = cont.iter();
^~~~~~~~~~~
/tmp/demo.rs:25:11: 25:19 error: mismatched types: expected `&<generic #1>` but found `(uint,uint)` (expected &-ptr but found tuple)
/tmp/demo.rs:25 check((3u, 5u));
^~~~~~~~
/tmp/demo.rs:25:5: 25:10 error: cannot determine a type for this bounded type parameter: unconstrained type
/tmp/demo.rs:25 check((3u, 5u));
^~~~~


% rustc --crate-type lib /tmp/demo.rs
/tmp/demo.rs:14:21: 14:32 error: cannot infer an appropriate lifetime for autoref due to conflicting requirements
/tmp/demo.rs:14 let cont_iter = cont.iter();
^~~~~~~~~~~
/tmp/demo.rs:14:21: 14:25 note: first, the lifetime cannot outlive the expression at 14:20...
/tmp/demo.rs:14 let cont_iter = cont.iter();
^~~~
/tmp/demo.rs:14:21: 14:25 note: ...so that automatically reference is valid at the time of borrow
/tmp/demo.rs:14 let cont_iter = cont.iter();
^~~~
/tmp/demo.rs:14:21: 14:32 note: but, the lifetime must be valid for the method call at 14:20...
/tmp/demo.rs:14 let cont_iter = cont.iter();
^~~~~~~~~~~
/tmp/demo.rs:14:21: 14:25 note: ...so that method receiver is valid for the method call
/tmp/demo.rs:14 let cont_iter = cont.iter();
^~~~
/tmp/demo.rs:25:11: 25:19 error: mismatched types: expected `&<generic #1>` but found `(uint,uint)` (expected &-ptr but found tuple)
/tmp/demo.rs:25 check((3u, 5u));
^~~~~~~~
/tmp/demo.rs:25:5: 25:10 error: cannot determine a type for this bounded type parameter: unconstrained type
/tmp/demo.rs:25 check((3u, 5u));
^~~~~
% 
@pnkfelix
Copy link
Member Author

(after further experimentation, I suspect the true issue here is that somewhere in the code there is an attempt to substitute {'r := 'a} but it is not traversing everywhere it should.

@edwardw
Copy link
Contributor

edwardw commented Mar 21, 2014

And it is an ICE if we do try to follow the lifetime hint:

...
fn check<'r, I: Iterator<uint>, T: Itble<'r, uint, I>>(cont: &'r T) -> bool
{
    // error: internal compiler error: cannot relate bound region: ReScope(83) <= ReEarlyBound(79, 0, r)
    let cont_iter = cont.iter();
    ...
}

@pnkfelix
Copy link
Member Author

@edwardw oh yes, I should have said, the reason I was looking at this program was to determine which variations of that ICE (see #5121) are still relevant

@ktt3ja
Copy link
Contributor

ktt3ja commented Mar 23, 2014

It looks like I forgot to traverse type parameter bound when rebuilding the function declaration. On the other hand, because of the ICE, it's unclear to me what the right error message should be when a type parameter bound is involved. My thought is that the implementation is kept as is until the ICE gets fixed. Meanwhile, if we see that a type parameter bound is involved, we would do one of the following:

  • Change the note to "consider using an explicit lifetime parameter" without giving any suggestion (easier)

or

  • Default back to the old note (harder)

@pnkfelix
Copy link
Member Author

@ktt3ja do you already have a patch that traverses the type parameter bound? You might as well share that so that we can try applying it, e.g. if i just hypothetically happen to be working on fixing the aforementioned ICE.

@ktt3ja
Copy link
Contributor

ktt3ja commented Mar 24, 2014

@pnkfelix: Not yet, but I'm working on it.

@ktt3ja
Copy link
Contributor

ktt3ja commented Mar 24, 2014

@pnkfelix: With PR #13112, this is the output for your example:

test.rs:22:1: 32:2 note: consider using an explicit lifetime parameter as shown: fn check<'a, I: Iterator<uint>, T: Itble<'a, uint, I>>(cont: &'a T) -> bool
test.rs:22 fn check<'r, I: Iterator<uint>, T: Itble<'r, uint, I>>(cont: &T) -> bool
test.rs:23 {
test.rs:24     let cont_iter = cont.iter();
test.rs:25     let result = cont_iter.fold(Some(0u16), |state, val| {
test.rs:26         state.map_or(None, |mask| {
test.rs:27             let bit = 1 << val;
           ...
test.rs:24:21: 24:32 error: cannot infer an appropriate lifetime for autoref due to conflicting requirements
test.rs:24     let cont_iter = cont.iter();
                               ^~~~~~~~~~~

@ghost ghost added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Oct 24, 2014
notriddle pushed a commit to notriddle/rust that referenced this issue Sep 20, 2022
Add a new configuration settings to set env vars when running cargo, rustc, etc. commands: cargo.extraEnv and checkOnSave.extraEnv

It can be extremely useful to be able to set environment variables when rust-analyzer is running various cargo or rustc commands (such as `cargo check`, `cargo --print cfg` or `cargo metadata`): users may want to set custom `RUSTFLAGS`, change `PATH` to use a custom toolchain or set a different `CARGO_HOME`.

There is the existing `server.extraEnv` setting that allows env vars to be set when the rust-analyzer server is launched, but using this as the recommended mechanism to also configure cargo/rust has some drawbacks:
- It convolutes configuring the rust-analyzer server with configuring cargo/rustc (one may want to change the `PATH` for cargo/rustc without affecting the rust-analyzer server).
- The name `server.extraEnv` doesn't indicate that cargo/rustc will be affected but renaming it to `cargo.extraEnv` doesn't indicate that the rust-analyzer server would be affected.
- To make the setting useful, it needs to be dynamically reloaded without requiring that the entire extension is reloaded. It might be possible to do this, but it would require the client communicating to the server what the overwritten env vars were at first launch, which isn't easy to do.

This change adds two new configuration settings: `cargo.extraEnv` and `checkOnSave.extraEnv` that can be used to change the environment for the rust-analyzer server after launch (thus affecting any process that rust-analyzer invokes) and the `cargo check` command respectively. `cargo.extraEnv` supports dynamic changes by keeping track of the pre-change values of environment variables, thus it can undo changes made previously before applying the new configuration (and then requesting a workspace reload).
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.

3 participants