Skip to content

Commit

Permalink
Change the ProgramDescription graph representation to use our porta…
Browse files Browse the repository at this point in the history
…ble `DefId`s (#45)

## What Changed?

This changes the representation of function ids from `Identifier` to
`DefId` and then exports additional information for each `DefId` as part
of the `ProgramDescription` that can be queried for debugging purposes.

It also changes the ids used in `GlobalLocation`s to `DefId` so
information about those functions can also be queried from the
`ProgramDescription`.

## Why Does It Need To?

This provides the necessary information to compose basic meaningful
error messages by allowing the graph nodes to be converted back into
function names and paths.

## Checklist

- [x] Above description has been filled out so that upon quash merge we
have a
  good record of what changed.
- [x] New functions, methods, types are documented. Old documentation is
updated
  if necessary
- [ ] Documentation in Notion has been updated
- [ ] Tests for new behaviors are provided
  - [ ] New test suites (if any) ave been added to the CI tests (in
`.github/workflows/rust.yml`) either as compiler test or integration
test.
*Or* justification for their omission from CI has been provided in this
PR
    description.
  • Loading branch information
JustusAdam committed Sep 25, 2023
1 parent 5dd907a commit db78aea
Show file tree
Hide file tree
Showing 26 changed files with 929 additions and 661 deletions.
21 changes: 21 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/paralegal-flow/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ serde_json = "1"
toml = "0.7"



# This is just for pinning this dependency
camino = "= 1.0.9"
serial_test = "2.0.0"
itertools = "0.11.0"
anyhow = "1.0.72"
thiserror = "1"

[build-dependencies]
chrono = "0.4"
Expand Down
3 changes: 1 addition & 2 deletions crates/paralegal-flow/src/ana/df.rs
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ impl<'a, 'tcx, 'inliner> Analysis<'tcx> for FlowAnalysis<'a, 'tcx, 'inliner> {

pub fn compute_flow_internal<'a, 'tcx, 's>(
tcx: TyCtxt<'tcx>,
body_id: BodyId,
def_id: DefId,
body_with_facts: &'a CachedSimplifedBodyWithFacts<'tcx>,
carries_marker: &'s InlineJudge<'tcx>,
) -> FlowResults<'a, 'tcx, 's> {
Expand All @@ -788,7 +788,6 @@ pub fn compute_flow_internal<'a, 'tcx, 's>(
// );
// debug!("{}", body_with_facts.simplified_body().to_string(tcx).unwrap());

let def_id = tcx.hir().body_owner_def_id(body_id).to_def_id();
let aliases = Aliases::build(tcx, def_id, body_with_facts.body_with_facts());
let location_domain = aliases.location_domain().clone();

Expand Down
20 changes: 9 additions & 11 deletions crates/paralegal-flow/src/ana/inline/graph.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use crate::{
ir::{regal, GlobalLocation},
mir, serde,
utils::{time, write_sep, DisplayViaDebug, FnResolution, IntoDefId, TinyBitSet},
BodyId, Either, HashMap, HashSet, Location, TyCtxt,
utils::{time, write_sep, DisplayViaDebug, FnResolution, TinyBitSet},
Either, HashMap, HashSet, Location, TyCtxt,
};

use super::algebra;

use paralegal_spdg::rustc_portable::DefId;
use petgraph::prelude as pg;

pub type ArgNum = u32;
Expand Down Expand Up @@ -275,12 +276,12 @@ impl<'tcx> InlinedGraph<'tcx> {

/// Construct the initial graph from a [`regal::Body`]
pub fn from_body(
body_id: BodyId,
def_id: DefId,
body: &regal::Body<'tcx, DisplayViaDebug<Location>>,
tcx: TyCtxt<'tcx>,
) -> Self {
time("Graph Construction From Regal Body", || {
let equations = to_global_equations(&body.equations, body_id);
let equations = to_global_equations(&body.equations);
let mut gwr = InlinedGraph {
equations,
graph: Default::default(),
Expand All @@ -301,11 +302,11 @@ impl<'tcx> InlinedGraph<'tcx> {
use regal::Target;
let from = match d {
Target::Call(c) => regal::SimpleLocation::Call((
GlobalLocation::single(**c, body_id),
GlobalLocation::single(**c, def_id),
*call_map.get(c).unwrap_or_else(|| {
panic!(
"Expected to find call at {c} in function {}",
tcx.def_path_debug_str(body_id.into_def_id(tcx))
tcx.def_path_debug_str(def_id)
)
}),
)),
Expand All @@ -322,7 +323,7 @@ impl<'tcx> InlinedGraph<'tcx> {
};

for (&loc, call) in body.calls.iter() {
let n = Node::Call((GlobalLocation::single(*loc, body_id), call.function));
let n = Node::Call((GlobalLocation::single(*loc, def_id), call.function));
for (idx, deps) in call.arguments.iter().enumerate() {
if let Some((_, deps)) = deps {
add_dep_edges(n, EdgeType::Data(idx as u32), deps)
Expand All @@ -341,10 +342,7 @@ impl<'tcx> InlinedGraph<'tcx> {
}

/// Globalize all locations mentioned in these equations.
fn to_global_equations(
eqs: &Equations<DisplayViaDebug<mir::Local>>,
_body_id: BodyId,
) -> Equations<GlobalLocal> {
fn to_global_equations(eqs: &Equations<DisplayViaDebug<mir::Local>>) -> Equations<GlobalLocal> {
eqs.iter()
.map(|eq| eq.map_bases(|target| GlobalLocal::at_root(**target)))
.collect()
Expand Down
83 changes: 33 additions & 50 deletions crates/paralegal-flow/src/ana/inline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use std::fmt::Write;

use crate::{
ana::algebra::{self, Term},
hir::BodyId,
ir::{
flows::CallOnlyFlow,
regal::{self, SimpleLocation},
Expand All @@ -28,13 +27,13 @@ use crate::{
rustc_target::abi::FieldIdx,
ty,
utils::{
body_name_pls, dump_file_pls, time, write_sep, DisplayViaDebug, FnResolution, IntoDefId,
IntoLocalDefId, Print, RecursionBreakingCache,
body_name_pls, dump_file_pls, time, write_sep, DisplayViaDebug, FnResolution, Print,
RecursionBreakingCache, TyCtxtExt,
},
AnalysisCtrl, DumpArgs, Either, HashMap, HashSet, MarkerCtx, Symbol, TyCtxt,
};

use rustc_utils::{cache::Cache, mir::borrowck_facts};
use rustc_utils::cache::Cache;

mod graph;
mod judge;
Expand Down Expand Up @@ -178,7 +177,7 @@ impl<'tcx> Inliner<'tcx> {
}
}

type BodyCache<'tcx> = Cache<BodyId, regal::Body<'tcx, DisplayViaDebug<Location>>>;
type BodyCache<'tcx> = Cache<DefId, regal::Body<'tcx, DisplayViaDebug<Location>>>;

/// Essentially just a bunch of caches of analyses.
pub struct Inliner<'tcx> {
Expand All @@ -187,7 +186,7 @@ pub struct Inliner<'tcx> {
/// Memoized graphs that have all their callees inlined. Unlike `base_memo`
/// this has to be recursion breaking, since a function may call itself
/// (possibly transitively).
inline_memo: RecursionBreakingCache<BodyId, InlinedGraph<'tcx>>,
inline_memo: RecursionBreakingCache<DefId, InlinedGraph<'tcx>>,
tcx: TyCtxt<'tcx>,
ana_ctrl: &'static AnalysisCtrl,
dbg_ctrl: &'static DumpArgs,
Expand Down Expand Up @@ -321,32 +320,22 @@ impl<'tcx> Inliner<'tcx> {
/// [`ProcedureGraph::from`]
fn get_procedure_graph<'a>(
&'a self,
body_id: BodyId,
def_id: DefId,
) -> &regal::Body<'tcx, DisplayViaDebug<Location>> {
self.base_memo.get(body_id, |bid| {
regal::compute_from_body_id(self.dbg_ctrl, bid, self.tcx, &self.marker_carrying)
self.base_memo.get(def_id, |_| {
regal::compute_from_def_id(self.dbg_ctrl, def_id, self.tcx, &self.marker_carrying)
})
}

/// Compute an inlined graph for this `body_id` (memoized)
pub fn get_inlined_graph(&self, body_id: BodyId) -> Option<&InlinedGraph<'tcx>> {
self.inline_memo.get(body_id, |bid| self.inline_graph(bid))
pub fn get_inlined_graph(&self, def_id: DefId) -> Option<&InlinedGraph<'tcx>> {
self.inline_memo.get(def_id, |bid| self.inline_graph(bid))
}

/// Convenience wrapper around [`Self::get_inlined_graph`]
fn get_inlined_graph_by_def_id(&self, def_id: LocalDefId) -> Option<&InlinedGraph<'tcx>> {
let hir = self.tcx.hir();
let body_id = match hir.maybe_body_owned_by(def_id) {
None => {
warn!(
"no body id for {:?}",
self.tcx.def_path_debug_str(def_id.into_def_id(self.tcx))
);
return None;
}
Some(b) => b,
};
self.get_inlined_graph(body_id)
let _hir = self.tcx.hir();
self.get_inlined_graph(def_id.to_def_id())
}

/// Make the set of equations relative to the call site described by `gli`
Expand Down Expand Up @@ -427,10 +416,8 @@ impl<'tcx> Inliner<'tcx> {
}
}

fn try_inline_as_async_fn(&self, i_graph: &mut InlinedGraph<'tcx>, body_id: BodyId) -> bool {
let local_def_id = body_id.into_local_def_id(self.tcx);
let body_with_facts =
borrowck_facts::get_simplified_body_with_borrowck_facts(self.tcx, local_def_id);
fn try_inline_as_async_fn(&self, i_graph: &mut InlinedGraph<'tcx>, def_id: DefId) -> bool {
let body_with_facts = self.tcx.body_for_def_id(def_id).unwrap();
let body = body_with_facts.simplified_body();
let num_args = body.args_iter().count();
// XXX This might become invalid if functions other than `async` can create generators
Expand Down Expand Up @@ -462,7 +449,7 @@ impl<'tcx> Inliner<'tcx> {
_ => unreachable!(),
};

let root_location = GlobalLocation::single(return_location, body_id);
let root_location = GlobalLocation::single(return_location, def_id);

// Following we must sumilate two code rewrites to the body of this
// function to simulate calling the closure. We make the closure
Expand Down Expand Up @@ -497,11 +484,11 @@ impl<'tcx> Inliner<'tcx> {

debug!(
"Recognized {} as an async function",
self.tcx.def_path_debug_str(local_def_id.to_def_id())
self.tcx.def_path_debug_str(def_id)
);
self.inline_one_function(
i_graph,
body_id,
def_id,
closure_fn.expect_local(),
&incoming,
&outgoing,
Expand All @@ -523,7 +510,7 @@ impl<'tcx> Inliner<'tcx> {
num_inlined,
max_call_stack_depth,
}: &mut InlinedGraph<'tcx>,
caller_function: BodyId,
caller_function: DefId,
inlining_target: LocalDefId,
incoming: &[(StdNode<'tcx>, Edge)],
outgoing: &[(StdNode<'tcx>, Edge)],
Expand Down Expand Up @@ -642,11 +629,11 @@ impl<'tcx> Inliner<'tcx> {
&self,
proc_g: &regal::Body<DisplayViaDebug<Location>>,
i_graph: &mut InlinedGraph<'tcx>,
body_id: BodyId,
def_id: DefId,
) -> EdgeSet<'tcx> {
let recursive_analysis_enabled = self.ana_ctrl.use_recursive_analysis();
let mut queue_for_pruning = HashSet::new();
if recursive_analysis_enabled && self.try_inline_as_async_fn(i_graph, body_id) {
if recursive_analysis_enabled && self.try_inline_as_async_fn(i_graph, def_id) {
return queue_for_pruning;
};
let targets = i_graph
Expand Down Expand Up @@ -718,7 +705,7 @@ impl<'tcx> Inliner<'tcx> {
.collect::<Vec<_>>();
self.inline_one_function(
i_graph,
body_id,
def_id,
did,
&incoming,
&outgoing,
Expand Down Expand Up @@ -779,44 +766,45 @@ impl<'tcx> Inliner<'tcx> {
/// In spite of the name of this function it not only inlines the graph but
/// also first creates it (with [`Self::get_procedure_graph`]) and globalize
/// it ([`to_global_graph`]).
fn inline_graph(&self, body_id: BodyId) -> InlinedGraph<'tcx> {
let proc_g = self.get_procedure_graph(body_id);
let mut gwr = InlinedGraph::from_body(body_id, proc_g, self.tcx);
fn inline_graph(&self, def_id: DefId) -> InlinedGraph<'tcx> {
let local_def_id = def_id.expect_local();
let proc_g = self.get_procedure_graph(def_id);
let mut gwr = InlinedGraph::from_body(def_id, proc_g, self.tcx);

let name = body_name_pls(self.tcx, body_id).name;
let name = body_name_pls(self.tcx, local_def_id).name;
if self.dbg_ctrl.dump_pre_inline_graph() {
dump_dot_graph(
dump_file_pls(self.tcx, body_id, "pre-inline.gv").unwrap(),
dump_file_pls(self.tcx, local_def_id, "pre-inline.gv").unwrap(),
&gwr,
)
.unwrap();
}
if self.dbg_ctrl.dump_local_equations() {
let mut eqout = dump_file_pls(self.tcx, body_id, "local.eqs").unwrap();
let mut eqout = dump_file_pls(self.tcx, local_def_id, "local.eqs").unwrap();
for eq in &gwr.equations {
use std::io::Write;
writeln!(eqout, "{eq}").unwrap();
}
}

let mut queue_for_pruning = time(&format!("Inlining subgraphs into '{name}'"), || {
self.perform_subfunction_inlining(proc_g, &mut gwr, body_id)
self.perform_subfunction_inlining(proc_g, &mut gwr, def_id)
});

if self.ana_ctrl.remove_inconsequential_calls().is_enabled() {
panic!("Removing inconsequential calls is no longer supported");
}

if self.dbg_ctrl.dump_global_equations() {
let mut eqout = dump_file_pls(self.tcx, body_id, "global.eqs").unwrap();
let mut eqout = dump_file_pls(self.tcx, local_def_id, "global.eqs").unwrap();
for eq in &gwr.equations {
use std::io::Write;
writeln!(eqout, "{eq}").unwrap();
}
}
if self.dbg_ctrl.dump_inlined_graph() {
dump_dot_graph(
dump_file_pls(self.tcx, body_id, "inlined.gv").unwrap(),
dump_file_pls(self.tcx, local_def_id, "inlined.gv").unwrap(),
&gwr,
)
.unwrap();
Expand All @@ -835,15 +823,10 @@ impl<'tcx> Inliner<'tcx> {
}
queue_for_pruning
};
self.prune_impossible_edges(
&mut gwr,
name,
&edges_to_prune,
body_id.into_local_def_id(self.tcx),
);
self.prune_impossible_edges(&mut gwr, name, &edges_to_prune, local_def_id);
if self.dbg_ctrl.dump_inlined_pruned_graph() {
dump_dot_graph(
dump_file_pls(self.tcx, body_id, "inlined-pruned.gv").unwrap(),
dump_file_pls(self.tcx, local_def_id, "inlined-pruned.gv").unwrap(),
&gwr,
)
.unwrap();
Expand Down
Loading

0 comments on commit db78aea

Please sign in to comment.