Skip to content

Commit

Permalink
Use instances in marker reachability. (#106)
Browse files Browse the repository at this point in the history
## What Changed?

Computing marker reachability now uses monomorphized instances that
properly propagate generic arguments.

## Why Does It Need To?

This reduces the unsoundness issues in this method. Before the generics
were basically ignored.

## 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 authored Nov 14, 2023
1 parent b9cf2ed commit fded811
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 21 deletions.
8 changes: 4 additions & 4 deletions crates/paralegal-flow/src/ana/inline/judge.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{mir::Place, utils::FnResolution, AnalysisCtrl, DefId, MarkerCtx, TyCtxt};
use crate::{mir::Place, utils::FnResolution, AnalysisCtrl, MarkerCtx, TyCtxt};

/// The interpretation of marker placement as it pertains to inlining and inline
/// elision.
Expand Down Expand Up @@ -40,7 +40,7 @@ impl<'tcx> InlineJudge<'tcx> {
) -> bool {
self.analysis_control.avoid_inlining()
&& !self.function_has_markers(function)
&& !self.marker_is_reachable(function.def_id())
&& !self.marker_is_reachable(function)
&& !self.probably_performs_side_effects(function, args, place_has_dependencies)
}

Expand Down Expand Up @@ -79,7 +79,7 @@ impl<'tcx> InlineJudge<'tcx> {
}

/// Is a marker reachable from this item?
fn marker_is_reachable(&self, def_id: DefId) -> bool {
self.marker_ctx.marker_is_reachable(def_id)
fn marker_is_reachable(&self, res: FnResolution<'tcx>) -> bool {
self.marker_ctx.marker_is_reachable(res)
}
}
2 changes: 1 addition & 1 deletion crates/paralegal-flow/src/ana/inline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ impl<'tcx> Inliner<'tcx> {
{
debug!("Inlining {function:?}");
return Some((id, *location, InlineAction::SimpleInline(local_id)));
} else if self.marker_carrying.marker_ctx().has_transitive_reachable_markers(def_id) {
} else if self.marker_carrying.marker_ctx().has_transitive_reachable_markers(*function) {
self.tcx.sess.struct_span_warn(
self.tcx.def_span(def_id),
"This function is not being inlined, but a marker is reachable from its inside.",
Expand Down
42 changes: 26 additions & 16 deletions crates/paralegal-flow/src/marker_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{
};
use rustc_utils::cache::CopyCache;

use std::rc::Rc;
use std::{borrow::Cow, rc::Rc};

type ExternalMarkers = HashMap<DefId, Vec<MarkerAnnotation>>;

Expand Down Expand Up @@ -136,28 +136,38 @@ impl<'tcx> MarkerCtx<'tcx> {
/// functions called in its body are marked.
///
/// XXX Does not take into account reachable type markers
pub fn marker_is_reachable(&self, def_id: DefId) -> bool {
self.is_marked(def_id) || self.has_transitive_reachable_markers(def_id)
pub fn marker_is_reachable(&self, res: FnResolution<'tcx>) -> bool {
self.is_marked(res.def_id()) || self.has_transitive_reachable_markers(res)
}

/// Queries the transitive marker cache.
pub fn has_transitive_reachable_markers(&self, def_id: DefId) -> bool {
pub fn has_transitive_reachable_markers(&self, res: FnResolution<'tcx>) -> bool {
self.db()
.marker_reachable_cache
.get_maybe_recursive(def_id, |_| self.compute_marker_reachable(def_id))
.get_maybe_recursive(res, |_| self.compute_marker_reachable(res))
.unwrap_or(false)
}

/// If the transitive marker cache did not contain the answer, this is what
/// computes it.
fn compute_marker_reachable(&self, def_id: DefId) -> bool {
let Some(body) = self.tcx().body_for_def_id_default_policy(def_id) else {
fn compute_marker_reachable(&self, res: FnResolution<'tcx>) -> bool {
let Some(body) = self.tcx().body_for_def_id_default_policy(res.def_id()) else {
return false;
};
let body = body.simplified_body();
body.basic_blocks
.iter()
.any(|bbdat| self.terminator_carries_marker(&body.local_decls, bbdat.terminator()))
body.basic_blocks.iter().any(|bbdat| {
let term = match res {
FnResolution::Final(inst) => {
Cow::Owned(inst.subst_mir_and_normalize_erasing_regions(
self.tcx(),
ty::ParamEnv::reveal_all(),
bbdat.terminator().clone(),
))
}
FnResolution::Partial(_) => Cow::Borrowed(bbdat.terminator()),
};
self.terminator_carries_marker(&body.local_decls, term.as_ref())
})
}

/// Does this terminator carry a marker?
Expand All @@ -166,19 +176,19 @@ impl<'tcx> MarkerCtx<'tcx> {
local_decls: &mir::LocalDecls,
terminator: &mir::Terminator<'tcx>,
) -> bool {
if let Ok((defid, _args, _)) = terminator.as_fn_and_args(self.tcx()) {
if let Ok((res, _args, _)) = terminator.as_instance_and_args(self.tcx()) {
debug!(
"Checking function {} for markers",
self.tcx().def_path_debug_str(defid)
self.tcx().def_path_debug_str(res.def_id())
);
if self.marker_is_reachable(defid) {
if self.marker_is_reachable(res) {
return true;
}
if let ty::TyKind::Alias(ty::AliasKind::Opaque, alias) =
local_decls[mir::RETURN_PLACE].ty.kind()
&& let ty::TyKind::Generator(closure_fn, _, _) = self.tcx().type_of(alias.def_id).skip_binder().kind() {
&& let ty::TyKind::Generator(closure_fn, substs, _) = self.tcx().type_of(alias.def_id).skip_binder().kind() {
return self.marker_is_reachable(
*closure_fn
FnResolution::Final(ty::Instance::expect_resolve(self.tcx(), ty::ParamEnv::reveal_all(), *closure_fn, substs))
);
}
}
Expand Down Expand Up @@ -248,7 +258,7 @@ pub struct MarkerDatabase<'tcx> {
local_annotations: HashMap<LocalDefId, Vec<Annotation>>,
external_annotations: ExternalMarkers,
/// Cache whether markers are reachable transitively.
marker_reachable_cache: CopyCache<DefId, bool>,
marker_reachable_cache: CopyCache<FnResolution<'tcx>, bool>,
/// Configuration options
config: &'static MarkerControl,
}
Expand Down
42 changes: 42 additions & 0 deletions guide/file-db-example/Cargo.lock

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

0 comments on commit fded811

Please sign in to comment.