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

Nightly regression: lifetime error with autoderef #28854

Closed
eefriedman opened this issue Oct 6, 2015 · 8 comments
Closed

Nightly regression: lifetime error with autoderef #28854

eefriedman opened this issue Oct 6, 2015 · 8 comments
Assignees
Labels
P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eefriedman
Copy link
Contributor

Testcase:

use std::collections::HashMap;
pub fn named_lints<'a>(names: &[&str],
                       transforms: &'a HashMap<&'static str, u32>)
                       -> Option<&'a u32> {
    transforms.get(&names[0])
}
fn main(){}
<anon>:5:20: 5:29 error: cannot infer an appropriate lifetime due to conflicting requirements [E0495]
<anon>:5     transforms.get(&names[0])
                            ^~~~~~~~~
<anon>:2:1: 6:2 help: consider using an explicit lifetime parameter as shown: fn named_lints<'a>(names: &[&'a str],
                   transforms: &'a HashMap<&'static str, u32>)
 -> Option<&'a u32>
<anon>:2 pub fn named_lints<'a>(names: &[&str],
<anon>:3                        transforms: &'a HashMap<&'static str, u32>)
<anon>:4                        -> Option<&'a u32> {
<anon>:5     transforms.get(&names[0])
<anon>:6 }
error: aborting due to previous error
playpen: application terminated with error code 101

It looks rather similar to #28839, but I'm not sure whether it's the same thing. Originally reported by @jonas-schievink (#28853 (comment)) .

@alexcrichton
Copy link
Member

triage: I-nominated

Tagging as T-compiler, but may also be T-lang (not sure). This compiles ok on stable/beta and fails to compile on nightly.

@alexcrichton alexcrichton added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 6, 2015
@sfackler sfackler added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Oct 6, 2015
@arielb1
Copy link
Contributor

arielb1 commented Oct 6, 2015

This and #28853 are the same issue.

The extraneous address-of operation (to names[0] here, caps.at() in #28854) causes the Q type parameter of HashMap::get to be inferred to &'α str, with the argument type being &'β &'α str, and no deref-coercion taking place, and the The T: Borrow<T> impl is selected.

As one of the T-s is &'static str, this code compiling is suspect. The only reason this code compiles is because the type of transforms is a supertype of &'a HashMap<&'c str, u32> (where 'c is the lifetime of the strings in names), which matches with 'α = 'c. This is, obviously, unsound, as get is allowed to assume that 'c: 'a - I am not sure how bad this can be with HashMap::get because of parametricity, but it would certainly be a hole with specialization.

@arielb1
Copy link
Contributor

arielb1 commented Oct 6, 2015

This problem is easily fixed by removing the extraneous &.

@aturon: I think it is cases like this that led to the removal of find_equiv etc.

@eefriedman
Copy link
Contributor Author

Oh... I see, this is kind of awkward: the signature is asserting that &'static str implements Borrow<&'a str>... which it doesn't, really. It implements Borrow<str> and Borrow<&'static str>.

This particular case is actually safe because &'static str could implement Borrow<&'a str>... the only problem is that it overlaps with impl<T> Borrow<T> for T.

@nikomatsakis
Copy link
Contributor

@arielb1

This is, obviously, unsound, as get is allowed to assume that 'c: 'a - I am not sure how bad this can be with HashMap::get because of parametricity, but it would certainly be a hole with specialization.

this is presumably #25860

@arielb1
Copy link
Contributor

arielb1 commented Oct 8, 2015

@nikomatsakis

Borrow is covariant in practice, so this doesn't actually form a soundness hole. #25860 involves contravariance which does not occur here.

@nikomatsakis
Copy link
Contributor

triage: P-high

@rust-highfive rust-highfive added P-high High priority and removed I-nominated labels Oct 15, 2015
@nikomatsakis
Copy link
Contributor

Closing in favor of #28853

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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