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

Implement friendly front-end format #45

Closed
wants to merge 5 commits into from
Closed

Conversation

lqd
Copy link
Member

@lqd lqd commented May 23, 2018

Implement the friendlier front-end format described in issue #26 — destined to help create isolated tests without needing rustc NLL facts, but still generating the same data structure.

Here's how it looks:

// program description
universal_regions { 'a, 'b, 'c }

// block description
block B0 {
    // effects are by default emitted on the Mid point
    invalidates(L0);

    // but we can still emit some at the Start point if needed
    // by separating the Start point effects from the Mid point effects with a "/"
    invalidates(L1) / kill(L2);

    goto B1;
}
block B1 {
    use('a), outlives('a: 'b), borrow_region_at('b, L1);
}

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, not quite there — do you think we should land and iterate in tree? Or would you prefer to hack on the PR a bit more?

src/program.rs Outdated
// the most common statement effects: mid point effects
for effect in &statement.effects {
match effect {
// facts: region_live_at(Region, Point)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so for region_live_at, we have to do more — we need to actually compute liveness I think, but I realize that to do that we need to also have def('a), of course (and maybe we should make both def and use accept multiple lifetimes). We could also try to capture the idea of variables with types, though that is creeping towards MIR and I think I had hoped to do that as a second pass.

In other words, as an example:

bb0 {
  def('a); // statement 0
  ; // statement 1
  goto bb1;
}

bb1 {
  ; // statement 0
  use('a); // statement 1
}

This ought to make 'a live at all the following points:

  • start(bb0/1),
  • mid(bb0/1),
  • start(bb1/0),
  • mid(bb1/0),
  • start(bb1/1),
  • mid(bb1/1).

The idea would be: some variable X whose type includes 'a is used at mid(bb1/1). So, 'a is live at that point, because the current value of X matters -- this flows backwards until we reach mid(bb0/0), at which point X was assigned, so its current value doesn't matter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can of course use datafrog to do this computation =) you can imagine us emitting use and def facts and setting up a rule:

region_live_at(R, P) :- use(R, P).
region_live_at(R, P) :-
  use(R, Q),
  cfg_edge(P, Q),
  !def(R, P).

Copy link
Contributor

@nikomatsakis nikomatsakis May 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also could potentially refactor the compiler to emit use and def facts and not region_live_at facts.

src/program.rs Outdated
facts.invalidates.insert((point, loan));
}

_ => panic!("Unexpected effect: {:?}", effect),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather factor Effect into nested enums so that this case never happens.

sapphire-arches and others added 5 commits May 24, 2018 14:15
Move AllFacts into polonius-engine

Remove accidentally commited timely code, update datafrog
Implement a friendlier front-end format destined to help create
isolated tests, without needing rustc NLL facts, but still generating
the same data structure.
@lqd
Copy link
Member Author

lqd commented May 24, 2018

Alright I'll fix this and reopen this later tonight

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

Successfully merging this pull request may close these issues.

3 participants