Skip to content

Commit

Permalink
Adjust Analysis/GenKillAnalysis interaction.
Browse files Browse the repository at this point in the history
`Analysis` has six methods of note:
- `apply_statement_effect`
- `apply_before_statement_effect`
- `apply_terminator_effect`
- `apply_before_terminator_effect`
- `apply_call_return_effect`
- `apply_switch_int_edge_effects`

`GenKillAnalysis` has six similar methods of note:
- `statement_effect`
- `before_statement_effect`
- `terminator_effect`
- `before_terminator_effect`
- `call_return_effect`
- `switch_int_edge_effects`

Every `GenKillAnalysis` also implicitly impls `Analysis`, thanks to
this blanket impl:
```
impl<'tcx, A> Analysis<'tcx> for A
where
    A: GenKillAnalysis<'tcx>,
    A::Domain: GenKill<A::Idx> + BitSetExt<A::Idx>,
```
which forwards `apply_statement_effect` to `statement_effect`, and so
on.

This is a nice symmetry, but it's misleading. The latter four
`GenKillAnalysis` methods (`terminator_effect`, ...) are only called
from the latter four `Analysis` methods (`apply_terminator_effect`,
...). In reality, to implement a `GenKillAnalysis` you want to implement
the first two `GenKillAnalysis` methods (which take `impl GenKill` args)
and the latter four `Analysis` methods (which can take `Self::Domain`
args).

This commit adjusts things accordingly.
- The latter four methods in `GenKillAnalysis` are removed.
- The blanket impl of `Analysis` for any type implementing
  `GenKillAnalysis` is removed.
- Each gen-kill analysis now implements the two `GenKillAnalysis`
  methods and the four latter `Analysis` methods.
- Each gen-kill analysis also implicitly implements the first two
  `Analysis` methods by using new macros
  `fn_apply_before_statement_effect_gen_kill!` and
  `fn_apply_statement_effect_gen_kill!` that just define a method that
  forwards on to the corresponding `GenKillAnalysis` method.

Overall this is a little less code and I find the whole structure easier
to understand.
  • Loading branch information
nnethercote committed Dec 5, 2023
1 parent 1b35beb commit db7352d
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 154 deletions.
24 changes: 15 additions & 9 deletions compiler/rustc_borrowck/src/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use rustc_middle::ty::RegionVid;
use rustc_middle::ty::TyCtxt;
use rustc_mir_dataflow::impls::{EverInitializedPlaces, MaybeUninitializedPlaces};
use rustc_mir_dataflow::ResultsVisitable;
use rustc_mir_dataflow::{self, fmt::DebugWithContext, GenKill};
use rustc_mir_dataflow::{Analysis, Direction, Results};
use rustc_mir_dataflow::{self, fmt::DebugWithContext, GenKill, GenKillAnalysis};
use rustc_mir_dataflow::{Analysis, AnalysisDomain, Direction, Results};
use std::fmt;

use crate::{places_conflict, BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext};
Expand Down Expand Up @@ -506,7 +506,7 @@ impl<'mir, 'tcx> Borrows<'mir, 'tcx> {
}
}

impl<'tcx> rustc_mir_dataflow::AnalysisDomain<'tcx> for Borrows<'_, 'tcx> {
impl<'tcx> AnalysisDomain<'tcx> for Borrows<'_, 'tcx> {
type Domain = BitSet<BorrowIndex>;

const NAME: &'static str = "borrows";
Expand All @@ -529,7 +529,7 @@ impl<'tcx> rustc_mir_dataflow::AnalysisDomain<'tcx> for Borrows<'_, 'tcx> {
/// region stops containing the CFG points reachable from the issuing location.
/// - we also kill loans of conflicting places when overwriting a shared path: e.g. borrows of
/// `a.b.c` when `a` is overwritten.
impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
impl<'tcx> GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
type Idx = BorrowIndex;

fn domain_size(&self, _: &mir::Body<'tcx>) -> usize {
Expand Down Expand Up @@ -592,17 +592,23 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
| mir::StatementKind::Nop => {}
}
}
}

impl<'tcx> Analysis<'tcx> for Borrows<'_, 'tcx> {
rustc_mir_dataflow::fn_apply_before_statement_effect_gen_kill!();

rustc_mir_dataflow::fn_apply_statement_effect_gen_kill!();

fn before_terminator_effect(
fn apply_before_terminator_effect(
&mut self,
trans: &mut impl GenKill<Self::Idx>,
trans: &mut Self::Domain,
_terminator: &mir::Terminator<'tcx>,
location: Location,
) {
self.kill_loans_out_of_scope_at_location(trans, location);
}

fn terminator_effect<'mir>(
fn apply_terminator_effect<'mir>(
&mut self,
trans: &mut Self::Domain,
terminator: &'mir mir::Terminator<'tcx>,
Expand All @@ -620,9 +626,9 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
terminator.edges()
}

fn call_return_effect(
fn apply_call_return_effect(
&mut self,
_trans: &mut impl GenKill<Self::Idx>,
_trans: &mut Self::Domain,
_block: mir::BasicBlock,
_return_places: CallReturnPlaces<'_, 'tcx>,
) {
Expand Down
143 changes: 34 additions & 109 deletions compiler/rustc_mir_dataflow/src/framework/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,9 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> {
/// functions for each statement in this way, the transfer function for an entire basic block can
/// be computed efficiently.
///
/// `Analysis` is automatically implemented for all implementers of `GenKillAnalysis`.
/// Use `fn_apply_statement_effect_gen_kill!` and (optionally)
/// `fn_apply_before_statement_effect_gen_kill!` to simplify the definition of
/// `Analysis` for analyses that also impl `GenKillAnalysis`.
pub trait GenKillAnalysis<'tcx>: Analysis<'tcx> {
type Idx: Idx;

Expand All @@ -275,117 +277,40 @@ pub trait GenKillAnalysis<'tcx>: Analysis<'tcx> {
_location: Location,
) {
}

/// See `Analysis::apply_terminator_effect`.
fn terminator_effect<'mir>(
&mut self,
trans: &mut Self::Domain,
terminator: &'mir mir::Terminator<'tcx>,
location: Location,
) -> TerminatorEdges<'mir, 'tcx>;

/// See `Analysis::apply_before_terminator_effect`.
fn before_terminator_effect(
&mut self,
_trans: &mut impl GenKill<Self::Idx>,
_terminator: &mir::Terminator<'tcx>,
_location: Location,
) {
}

/* Edge-specific effects */

/// See `Analysis::apply_call_return_effect`.
fn call_return_effect(
&mut self,
trans: &mut impl GenKill<Self::Idx>,
block: BasicBlock,
return_places: CallReturnPlaces<'_, 'tcx>,
);

/// See `Analysis::apply_switch_int_edge_effects`.
fn switch_int_edge_effects<G: GenKill<Self::Idx>>(
&mut self,
_block: BasicBlock,
_discr: &mir::Operand<'tcx>,
_edge_effects: &mut impl SwitchIntEdgeEffects<G>,
) {
}
}

impl<'tcx, A> Analysis<'tcx> for A
where
A: GenKillAnalysis<'tcx>,
A::Domain: GenKill<A::Idx> + BitSetExt<A::Idx>,
{
fn apply_statement_effect(
&mut self,
state: &mut A::Domain,
statement: &mir::Statement<'tcx>,
location: Location,
) {
self.statement_effect(state, statement, location);
}

fn apply_before_statement_effect(
&mut self,
state: &mut A::Domain,
statement: &mir::Statement<'tcx>,
location: Location,
) {
self.before_statement_effect(state, statement, location);
}

fn apply_terminator_effect<'mir>(
&mut self,
state: &mut A::Domain,
terminator: &'mir mir::Terminator<'tcx>,
location: Location,
) -> TerminatorEdges<'mir, 'tcx> {
self.terminator_effect(state, terminator, location)
}

fn apply_before_terminator_effect(
&mut self,
state: &mut A::Domain,
terminator: &mir::Terminator<'tcx>,
location: Location,
) {
self.before_terminator_effect(state, terminator, location);
}

/* Edge-specific effects */

fn apply_call_return_effect(
&mut self,
state: &mut A::Domain,
block: BasicBlock,
return_places: CallReturnPlaces<'_, 'tcx>,
) {
self.call_return_effect(state, block, return_places);
}

fn apply_switch_int_edge_effects(
&mut self,
block: BasicBlock,
discr: &mir::Operand<'tcx>,
edge_effects: &mut impl SwitchIntEdgeEffects<A::Domain>,
) {
self.switch_int_edge_effects(block, discr, edge_effects);
}
/// Use this to define `Analysis::apply_before_statement_effect` in types that
/// impl both `Analysis` and `GenKillAnalysis`. It delegates to
/// `GenKillAnalysis::apply_before_statement_effect`.
#[macro_export]
macro_rules! fn_apply_before_statement_effect_gen_kill {
() => {
fn apply_before_statement_effect(
&mut self,
trans: &mut Self::Domain,
stmt: &rustc_middle::mir::Statement<'tcx>,
location: Location,
) {
self.before_statement_effect(trans, stmt, location);
}
};
}

/* Extension methods */
#[inline]
fn into_engine<'mir>(
self,
tcx: TyCtxt<'tcx>,
body: &'mir mir::Body<'tcx>,
) -> Engine<'mir, 'tcx, Self>
where
Self: Sized,
{
Engine::new_gen_kill(tcx, body, self)
}
/// Use this to define `Analysis::apply_statement_effect` in types that impl
/// both `Analysis` and `GenKillAnalysis`. It just delegates to
/// `GenKillAnalysis::apply_statement_effect`.
#[macro_export]
macro_rules! fn_apply_statement_effect_gen_kill {
() => {
fn apply_statement_effect(
&mut self,
trans: &mut Self::Domain,
stmt: &rustc_middle::mir::Statement<'tcx>,
location: Location,
) {
self.statement_effect(trans, stmt, location);
}
};
}

/// The legal operations for a transfer function in a gen/kill problem.
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rustc_index::bit_set::BitSet;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::*;

use crate::{AnalysisDomain, GenKill, GenKillAnalysis};
use crate::{Analysis, AnalysisDomain, GenKill, GenKillAnalysis};

/// A dataflow analysis that tracks whether a pointer or reference could possibly exist that points
/// to a given local. This analysis ignores fake borrows, so it should not be used by
Expand Down Expand Up @@ -49,8 +49,12 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeBorrowedLocals {
) {
self.transfer_function(trans).visit_statement(statement, location);
}
}

impl<'tcx> Analysis<'tcx> for MaybeBorrowedLocals {
crate::fn_apply_statement_effect_gen_kill!();

fn terminator_effect<'mir>(
fn apply_terminator_effect<'mir>(
&mut self,
trans: &mut Self::Domain,
terminator: &'mir Terminator<'tcx>,
Expand All @@ -60,9 +64,9 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeBorrowedLocals {
terminator.edges()
}

fn call_return_effect(
fn apply_call_return_effect(
&mut self,
_trans: &mut impl GenKill<Self::Idx>,
_trans: &mut Self::Domain,
_block: BasicBlock,
_return_places: CallReturnPlaces<'_, 'tcx>,
) {
Expand Down
Loading

0 comments on commit db7352d

Please sign in to comment.