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

hr_lifetime_in_assoc_type future-compatibility warnings #33685

Closed
nikomatsakis opened this issue May 17, 2016 · 12 comments
Closed

hr_lifetime_in_assoc_type future-compatibility warnings #33685

nikomatsakis opened this issue May 17, 2016 · 12 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented May 17, 2016

This is the summary issue for the hr_lifetime_in_assoc_type future-compatibility warning and other related errors. The goal of this page is describe why this change was made and how you can fix code that is affected by it. It also provides a place to ask questions or register a complaint if you feel the change should not be made. For more information on the policy around future-compatibility warnings, see our [breaking change policy guidelines][].

What is the warning for?

The warning is issued in two cases. First, if you have a where-clause where a higher-ranked lifetime appears only in the associated type. This most commonly occurs when the lifetime appears in the return type for one of the closure traits:

where F: for<'a> Fn() -> &'a u32
//                        ^^ this one

But could also occur in other associated type bindings:

where T: for<'a> Iterator<Item=&'a u32>
//                              ^^ this one

The second situation is when you have a fn type with a lifetime that appears only in the return type:

let x: for<'a> fn() -> &'a i32
//                      ^^ this one

How to fix

Note that it is perfectly fine for a higher-ranked lifetime to appear in an associated type or function return type if it also appears in the trait inputs or function arguments:

where F: for<'a> Fn(&'a u32) -> &'a u32 // OK!
//                   ^^ appears in trait input types here

where for<'a> &'a T: Iterator<Item=&'a u32> // OK!
//             ^^ appears in trait input types here

let x: for<'a> fn(&'a i32) -> &'a i32
//                 ^^ appears in function arguments here

In cases where the lifetime appears only in the return type, it is also generally better to use 'static:

// so e.g. this:
where F: for<'a> Fn() -> &'a u32

// becomes this:
where F: Fn() -> &'static u32

More details and future errors

These constructs are being rejected as part of the fix for issue #32330. This is a soundness bug that concerns cases like these. In general, all higher-ranked lifetimes that are used in the value of an associated type (and in this case, the function return type is an associated type) must also appear in the trait's input types. This constraint is already enforced for impls, but enforcement was overlooked for where-clauses.

These sorts of where-clauses and types are not only ill-formed, they are useless. For example, now that #32330 is fixed, there are no types that could satisfy such a where-clause, nor are there any functions with a type like for<'a> fn() -> &'a i32 (see next section for more details).

Once #32330 is fully fixed, there are some cases of code that will yield errors which will not currently receive warnings. The crux of the problem concerns lifetime parameters that only appear in the return type of a fn declaration -- these are changing to no longer be considered "higher-ranked" (or "late-bound"). This change is usually harmless, but it can trigger compilation errors of various kinds. For engineering reasons, there are some cases where it was not possible to issue warnings, though we will try to cause the error message to direct people to this issue.

Here is an example of an affected function:

fn foo<'a>() -> &'a i32 { }
//               ^^ will no be longer higher-ranked

Merely calling this function should work as before. But assigning it to a fn pointer variable will trigger an error, depending on the type of that variable:

fn main() {
    let f: for<'a> fn() -> &'a i32 = foo;
    //                      ^^ note that we will now issue a warning for this type, as well.
}

Another, more subtle, error can occur when a function is stored into a variable and then invoked multiple times:

fn bar<'b, 'c>() -> (&'b i32, 'c i32) {
    let f = foo;
    (f(), f()) // <-- will be an error
    // but note that (foo(), foo()) works
}

Essentially, the change is that each time you reference foo (directly!), there is now a single lifetime assigned for its parameter 'a, no matter how many times you call it. In cases like bar above, we need to assign distinct references to each call, so we have to reference foo twice.

Related bugs

Also related is #33684, which can cause these warnings to be issued more frequently than one might expect. In particular, due to #33684, the compiler fails to recognize certain equivalences around lifetime parameters. For example, this function:

fn foo<'a>(&u32) -> &'a u32

probably ought to be considered equivalent to this function:

fn foo(&u32) -> &'static u32

and both of them ought to be usable in a context where for<'a> fn(&'a u32) -> &'a u32 is required. But because of #33684, only the version that explicitly uses 'static is considered to satisfy the higher-ranked type.

When will this warning become a hard error?

The typical process is as follows: at the beginning of each 6-week release cycle, the Rust compiler team will review the set of outstanding future compatibility warnings and nominate some of them for Final Comment Period. Toward the end of the cycle, we will review any comments and make a final determination whether to convert the warning into a hard error or remove it entirely.

@nikomatsakis nikomatsakis added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. and removed B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. labels May 17, 2016
@nikomatsakis
Copy link
Contributor Author

Let's move this to Deny by default.

@nikomatsakis
Copy link
Contributor Author

@rfcbot fcp close

I move that we make this a hard error. This is a soundness-related change. The change to Deny is in PR #37843 and is currently being tested for impact.

@rfcbot
Copy link

rfcbot commented Nov 17, 2016

Team member @nikomatsakis has proposed to close this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

bors added a commit that referenced this issue Nov 22, 2016
make HR_LIFETIME_IN_ASSOC_TYPE deny-by-default

It's time to fix issue #32330.

cc #33685
cc @arielb1
@brson brson added P-medium Medium priority and removed P-high High priority labels Dec 1, 2016
@vadimcn
Copy link
Contributor

vadimcn commented Dec 20, 2016

@nikomatsakis:

 // so e.g. this:
 where F: for<'a> Fn() -> &'a u32
 
 // becomes this:
 where F: Fn() -> &'static u32```

What if the returned value lives in the closure's environment? I don't think 'static would be a correct lifetime in such a case, but is there a syntax for referring to the lifetime of closure's self?

@eddyb
Copy link
Member

eddyb commented Dec 20, 2016

@vadimcn fn foo<'a, F: Fn() -> &'a u32>(f: &'a F)

@aturon
Copy link
Member

aturon commented Jan 6, 2017

Is this ready to be closed?

@nikomatsakis
Copy link
Contributor Author

@aturon it is deny-by-default now, but I have not yet opened the PR to remove the underlying problem. It is all but ready though, hopefully I will get to it today.

@rfcbot
Copy link

rfcbot commented Jan 23, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

psst @nikomatsakis, I wasn't able to add the final-comment-period label, please do so.

@rfcbot
Copy link

rfcbot commented Feb 2, 2017

The final comment period is now complete.

@arielb1
Copy link
Contributor

arielb1 commented Apr 2, 2017

No more compat warnings since #38897.

@823984418
Copy link
Contributor

I think the reason for #32330 is Output is an association type.

pub trait FnOnce<Args: Tuple> {
    type Output;
    fn call_once(self, args: Args) -> Self::Output;
}

Why the Output is an association type and not a generic type?

pub trait FnOnce<Args: Tuple, Output> {
    fn call_once(self, args: Args) -> Output;
}

In my opinion, function pointers are similar to:

pub struct r#fn<Args: Tuple, Output>

This shows that the Output works well as a generic parameter.


Also related is #33684, which can cause these warnings to be issued more frequently than one might expect. In particular, due to #33684, the compiler fails to recognize certain equivalences around lifetime parameters. For example, this function:

fn foo<'a>(&u32) -> &'a u32

probably ought to be considered equivalent to this function:

fn foo(&u32) -> &'static u32

So what if

fn foo<'a>() -> Vec<&'a i32>

rewrite into

fn foo() -> Vec<&'static i32>

And then only to push &'static i32 ?

@823984418
Copy link
Contributor

If the Output changes as generic parameters, appears in the input requirements are not necessary, because in addition to the function.

trait A {
    type Output;
}

No same type can implement different lifecycle Output
So the Output as an association type is just an error。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. P-medium Medium priority 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

8 participants