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

Revisit the "Location::All" hack #24

Open
nikomatsakis opened this issue May 12, 2018 · 2 comments
Open

Revisit the "Location::All" hack #24

nikomatsakis opened this issue May 12, 2018 · 2 comments

Comments

@nikomatsakis
Copy link
Contributor

As part of the nll-facts borrowck work, I added the idea of outlives relation that occur at all points. I did my best to explain it here:

https://github.com/rust-lang/rust/blob/0cd465087d800f6e1183ed3e8a71262039245902/src/librustc_mir/borrow_check/nll/type_check/mod.rs#L627-L657

I handled this by adding the relevant outlives relationships to all points. But, looking at the outlives.facts I saw that these relations account for a LOT of noise. Of the ~500K tuples in that file, about half are due to this. =) Moreover, these tuples will mess up any optimizations like #20. It's clear we need a better approach, but I'm not sure what.

That said, manually removing those tuples and re-running the analysis shows that they only have a small effect on overall performance (from ~40s to ~35s).

@nikomatsakis
Copy link
Contributor Author

In my branch nll-alias-analysis-no-loc-call, I take a different approach that removes Location::All. However, I suspect it will have some "not obviously desirable" effects.

In particular, if you "reassign" a variable that has a type annotation, you get "fresh regions":

fn foo(mut x: &'static u32) {
  let y = 22;
  x = &y; // OK
  // ... here, `x` is not considered `'static` ..
}

This falls out from trying to be smart about examples like:

let mut p = &x;
let q = p;
p = &y;

I'm inclined to think we should run with it for now, but we'll have to revisit this issue prior to stabilization.

@nikomatsakis
Copy link
Contributor Author

Preliminary PR posted in rustc to change how our input facts are organized:

rust-lang/rust#50938

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant