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

Complete Dart slice borrowing #437

Merged
merged 14 commits into from
Mar 5, 2024
14 changes: 14 additions & 0 deletions core/src/hir/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ impl LifetimeEnv {
(0..self.num_lifetimes()).map(Lifetime::new)
}

/// Get the bounds for a named lifetime (none for unnamed lifetimes)
pub(super) fn get_bounds(&self, named_lifetime: Lifetime) -> Option<&BoundedLifetime> {
self.nodes.get(named_lifetime.0)
}

/// Returns a new [`LifetimeEnv`].
pub(super) fn new(
nodes: SmallVec<[BoundedLifetime; INLINE_NUM_LIFETIMES]>,
Expand Down Expand Up @@ -386,6 +391,15 @@ impl<'def, 'tcx> LinkedLifetimes<'def, 'tcx> {
}
}

/// Takes a lifetime at the def site and produces one at the use site
pub fn def_to_use(&self, def_lt: Lifetime) -> MaybeStatic<Lifetime> {
*self
.lifetimes
.as_slice()
.get(def_lt.0)
.expect("All def site lifetimes must be used!")
}

/// The lifetime env at the def site. Def lifetimes should be resolved
/// against this.
pub fn def_env(&self) -> &'tcx LifetimeEnv {
Expand Down
14 changes: 14 additions & 0 deletions core/src/hir/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,20 @@ impl ReturnType {

set
}

pub fn with_contained_types(&self, mut f: impl FnMut(&OutType)) {
match self {
Self::Infallible(SuccessType::OutType(o))
| Self::Nullable(SuccessType::OutType(o))
| Self::Fallible(SuccessType::OutType(o), None) => f(o),
Self::Fallible(SuccessType::OutType(o), Some(o2)) => {
f(o);
f(o2)
}
Self::Fallible(_, Some(o)) => f(o),
_ => (),
}
}
}

impl ParamSelf {
Expand Down
205 changes: 190 additions & 15 deletions core/src/hir/methods/borrowing_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,88 @@
//! This is useful for backends which wish to figure out the borrowing relationships between parameters
//! and return values,
//! and then delegate how lifetimes get mapped to fields to the codegen for those types respectively.
//!
//!
//! # Typical usage
//!
//! In managed languages, to ensure the GC doesn't prematurely clean up the borrowed-from object,
//! we use the technique of stashing a borrowed-from object as a field of a borrowed-to object.
//! This is called a "lifetime edge" (since it's an edge in the GC graph). An "edge array" is typically
//! a stash of all the edges on an object, or all the edges corresponding to a particular lifetime.
//!
//! This stashing is primarily driven by method codegen, which is where actual borrow relationships can be set up.
//!
//! Typically, codegen for a method should instantiate a [`BorrowedParamVisitor`] and uses it to [`BorrowedParamVisitor::visit_param()`] each input parameter to determine if it needs to be linked into an edge.
//! Whilst doing so, it may return useful [`ParamBorrowInfo`] which provides additional information on handling structs and slices. More on structs later.
//! At the end of the visiting, [`BorrowedLifetimeInfo::borrow_map()`] can be called to get a list of relevant input edges: each lifetime that participates in borrowing,
//! and a list of parameter names that it borrows from.
//!
//! This visitor will automatically handle transitive lifetime relationships as well.
//!
//! These edges should be put together into "edge arrays" which then get passed down to output types, handling transitive lifetimes as necessary.
//!
//! ## Method codegen for slices
//!
//! Slices are strange: in managed languages a host slice will not be a Rust slice, unlike other borrowed host values (a borrowed host opaque is just a borrowed Rust
//! opaque). We need to convert these across the boundary by allocating a temporary arena or something.
//!
//! If the slice doesn't participate in borrowing, this is easy: just allocate a temporary arena. However, if not, we need to allocate an arena with a finalizer (or something similar)
//! and add that arena to the edge instead. [`LifetimeEdgeKind`] has special handling for this.
//!
//! Whether or not the slice participates in borrowing is found out from the [`ParamBorrowInfo`] returned by [`BorrowedParamVisitor::visit_param()`].
//!
//! # Method codegen for struct params
//!
//! Structs can contain many lifetimes and have relationships between them. Generally, a Diplomat struct is a "bag of stuff"; it is converted to a host value that is structlike, with fields
//! individually converted.
//!
//! Diplomat enforces that struct lifetime bounds never imply additional bounds on methods during HIR validation, so the relationships between struct
//! lifetimes are not relevant for code here (and as such you should hopefully never need to call [`LifetimeEnv::all_longer_lifetimes()`])
//! for a struct env).
//!
//! The borrowing technique in this module allows a lot of things to be delegated to structs. As will be explained below, structs will have:
//!
//! - A `fields_for_lifetime_foo()` set of methods that returns all non-slice fields corresponding to a lifetime
//! - "append array" outparams for stashing edges when converting from host-to-Rust
//! - Some way of passing down edge arrays to individual borrowed fields when constructing Rust-to-host
//!
//! In methods, when constructing the edge arrays, `fields_for_lifetime_foo()` for every borrowed param can be appended to each
//! one whenever [`LifetimeEdgeKind::StructLifetime`] asks you to do so. The code needs to handle lifetime
//! transitivity, since the struct will not be doing so. Fortunately the edges produced by [`BorrowedParamVisitor`] already do so.
//!
//! Then, when converting structs host-to-Rust, every edge array relevant to a struct lifetime should be passed in for an append array. Append arrays
//! are nested arrays: they are an array of edge arrays. The struct will append further things to the edge array.
//!
//! Finally, when converting Rust-to-host, if producing a struct, make sure to pass in all the edge arrays.
//!
//! # Struct codegen
//!
//! At a high level, struct codegen needs to deal with lifetimes in three places:
//!
//! - A `fields_for_lifetime_foo()` set of methods that returns all non-slice fields corresponding to a lifetime
//! - "append array" outparams for stashing edges when converting from host-to-Rust
//! - Some way of passing down edge arrays to individual borrowed fields when constructing Rust-to-host
//!
//! The backend may choose to handle just slices via append arrays, or handle all borrowing there, in which case `fields_for_lifetime_foo()` isn't necessary.
//!
//! `fields_for_lifetime_foo()` should return an iterator/list/etc corresponding to each field that *directly* borrows from lifetime foo.
//! This may include calling `fields_for_lifetime_bar()` on fields that are themselves structs. As mentioned previously, lifetime relationships
//! are handled by methods and don't need to be dealt with here. There are no helpers for doing this but it's just a matter of
//! filtering for fields using the lifetime, except for handling nested structs ([`StructBorrowInfo::compute_for_struct_field()`])
//!
//! Append arrays are similarly straightforward: for each lifetime on the struct, the host-to-Rust constructor should accept a list of edge arrays. For slice fields,
//! if the appropriate list is nonempty, the slice's arena edge should be appended to all of the edge arrays in it. These arrays should also be passed down to struct fields
//! appropriately: [`StructBorrowInfo::compute_for_struct_field()`] is super helpful for getting a list edges something should get passed down to.
//!
//! Finally, when converting Rust-to-host, the relevant lifetime edges should be proxied down to the final host type constructors who can stash them wherever needed.
//! This is one again a matter of filtering fields by the lifetimes they use, and for nested structs you can use [`StructBorrowInfo::compute_for_struct_field()`].
Copy link
Collaborator

Choose a reason for hiding this comment

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

praise: nice explanation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really felt that the existing code was confusing partly because there was nothing telling you where to start. I might document the borowing_fields approach some day.


use std::collections::{BTreeMap, BTreeSet};

use crate::hir::{self, Method, TypeContext};
use crate::hir::{self, Method, StructDef, TyPosition, TypeContext};

use crate::hir::lifetimes::{Lifetime, LifetimeEnv, MaybeStatic};
use crate::hir::ty_position::StructPathLike;

/// A visitor for processing method parameters/returns and understanding their borrowing relationships, shallowly.
///
Expand All @@ -19,8 +95,6 @@ use crate::hir::lifetimes::{Lifetime, LifetimeEnv, MaybeStatic};
/// Obtain from [`Method::borrowing_param_visitor()`].
pub struct BorrowingParamVisitor<'tcx> {
tcx: &'tcx TypeContext,
#[allow(unused)] // to be used later
method: &'tcx Method,
used_method_lifetimes: BTreeSet<Lifetime>,
borrow_map: BTreeMap<Lifetime, BorrowedLifetimeInfo<'tcx>>,
}
Expand Down Expand Up @@ -78,7 +152,6 @@ impl<'tcx> BorrowingParamVisitor<'tcx> {
.collect();
BorrowingParamVisitor {
tcx,
method,
used_method_lifetimes,
borrow_map,
}
Expand All @@ -94,23 +167,28 @@ impl<'tcx> BorrowingParamVisitor<'tcx> {
self.borrow_map
}

/// Processes a parameter, adding it to the borrow_map for any lifetimes it references. Returns true if the lifetime is referenced.
/// Processes a parameter, adding it to the borrow_map for any lifetimes it references. Returns further information about the type of borrow.
///
/// This basically boils down to: For each lifetime that is actually relevant to borrowing in this method, check if that
/// lifetime or lifetimes longer than it are used by this parameter. In other words, check if
/// it is possible for data in the return type with this lifetime to have been borrowed from this parameter.
/// If so, add code that will yield the ownership-relevant parts of this object to incoming_edges for that lifetime.
pub fn visit_param(&mut self, ty: &hir::Type, param_name: &str) -> bool {
pub fn visit_param(&mut self, ty: &hir::Type, param_name: &str) -> ParamBorrowInfo<'tcx> {
let mut is_borrowed = false;
if self.used_method_lifetimes.is_empty() {
return false;
if let hir::Type::Slice(..) = *ty {
return ParamBorrowInfo::TemporarySlice;
} else {
return ParamBorrowInfo::NotBorrowed;
}
}
let mut edges_pushed = false;

// Structs have special handling: structs are purely Dart-side, so if you borrow
// from a struct, you really are borrowing from the internal fields.
if let hir::Type::Struct(s) = ty {
let mut borrowed_struct_lifetime_map = BTreeMap::<Lifetime, BTreeSet<Lifetime>>::new();
let link = s.link_lifetimes(self.tcx);
for method_lifetime in self.borrow_map.values_mut() {
for (method_lifetime, method_lifetime_info) in &mut self.borrow_map {
// Note that ty.lifetimes()/s.lifetimes() is lifetimes
// in the *use* context, i.e. lifetimes on the Type that reference the
// indices of the method's lifetime arrays. Their *order* references
Expand All @@ -124,19 +202,33 @@ impl<'tcx> BorrowingParamVisitor<'tcx> {
// This is a struct so lifetimes_def_only() is fine to call
for (use_lt, def_lt) in link.lifetimes_def_only() {
if let MaybeStatic::NonStatic(use_lt) = use_lt {
if method_lifetime.all_longer_lifetimes.contains(&use_lt) {
if method_lifetime_info.all_longer_lifetimes.contains(&use_lt) {
let edge = LifetimeEdge {
param_name: param_name.into(),
kind: LifetimeEdgeKind::StructLifetime(link.def_env(), def_lt),
};
method_lifetime.incoming_edges.push(edge);
edges_pushed = true;
method_lifetime_info.incoming_edges.push(edge);

is_borrowed = true;

borrowed_struct_lifetime_map
.entry(def_lt)
.or_default()
.insert(*method_lifetime);
// Do *not* break the inner loop here: even if we found *one* matching lifetime
// in this struct that may not be all of them, there may be some other fields that are borrowed
}
}
}
}
if is_borrowed {
ParamBorrowInfo::Struct(StructBorrowInfo {
env: link.def_env(),
borrowed_struct_lifetime_map,
})
} else {
ParamBorrowInfo::NotBorrowed
}
} else {
for method_lifetime in self.borrow_map.values_mut() {
for lt in ty.lifetimes() {
Expand All @@ -154,16 +246,99 @@ impl<'tcx> BorrowingParamVisitor<'tcx> {
};

method_lifetime.incoming_edges.push(edge);
edges_pushed = true;
is_borrowed = true;
// Break the inner loop: we've already determined this
break;
}
}
}
}
match (is_borrowed, ty) {
(true, &hir::Type::Slice(..)) => ParamBorrowInfo::BorrowedSlice,
(false, &hir::Type::Slice(..)) => ParamBorrowInfo::TemporarySlice,
(false, _) => ParamBorrowInfo::NotBorrowed,
(true, _) => ParamBorrowInfo::BorrowedOpaque,
}
}
}
}

// return true if the number of edges changed
edges_pushed
/// Information relevant to borrowing for producing conversions
#[derive(Clone, Debug)]
#[non_exhaustive]
pub enum ParamBorrowInfo<'tcx> {
/// No borrowing constraints. This means the parameter
/// is not borrowed by the output and also does not need temporary borrows
NotBorrowed,
/// A slice that is not borrowed by the output (but will still need temporary allocation)
TemporarySlice,
/// A slice that is borrowed by the output
BorrowedSlice,
/// A struct parameter that is borrowed by the output
Struct(StructBorrowInfo<'tcx>),
/// An opaque type that is borrowed
BorrowedOpaque,
}

/// Information about the lifetimes of a struct parameter that are borrowed by a method output or by a wrapping struct
#[derive(Clone, Debug)]
#[non_exhaustive]
pub struct StructBorrowInfo<'tcx> {
/// This is the struct's lifetime environment
pub env: &'tcx LifetimeEnv,
/// A map from (borrow-relevant) struct lifetimes to lifetimes in the method (or wrapping struct) that may flow from it
pub borrowed_struct_lifetime_map: BTreeMap<Lifetime, BTreeSet<Lifetime>>,
}

impl<'tcx> StructBorrowInfo<'tcx> {
/// Get borrowing info for a struct field, if it does indeed borrow
///
/// The lifetime map produced here does not handle lifetime dependencies: the expectation is that the struct
/// machinery generated by this will be called by method code that handles these dependencies. We try to handle
/// lifetime dependencies in ONE place.
pub fn compute_for_struct_field<P: TyPosition>(
struc: &StructDef<P>,
field: &P::StructPath,
tcx: &'tcx TypeContext,
) -> Option<Self> {
if field.lifetimes().as_slice().is_empty() {
return None;
}

let mut borrowed_struct_lifetime_map = BTreeMap::<Lifetime, BTreeSet<Lifetime>>::new();

let link = field.link_lifetimes(tcx);

for outer_lt in struc.lifetimes.all_lifetimes() {
// Note that field.lifetimes()
// in the *use* context, i.e. lifetimes on the Type that reference the
// indices of the outer struct's lifetime arrays. Their *order* references
// the indices of the underlying struct def. We need to link the two,
// since the _fields_for_lifetime_foo() methods are named after
// the *def* context lifetime.
//
// This is a struct so lifetimes_def_only() is fine to call
for (use_lt, def_lt) in link.lifetimes_def_only() {
if let MaybeStatic::NonStatic(use_lt) = use_lt {
// We do *not* need to transitively check for longer lifetimes here:
//
if outer_lt == use_lt {
borrowed_struct_lifetime_map
.entry(def_lt)
.or_default()
.insert(outer_lt);
}
}
}
}
if borrowed_struct_lifetime_map.is_empty() {
// if the inner struct is only statics
None
} else {
Some(StructBorrowInfo {
env: link.def_env(),
borrowed_struct_lifetime_map,
})
}
}
}
23 changes: 0 additions & 23 deletions core/src/hir/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,6 @@ impl ReturnableStructPath {
Self::OutStruct(p) => &p.lifetimes,
}
}

/// Get a map of lifetimes used on this path to lifetimes as named in the def site. See [`LinkedLifetimes`]
/// for more information.
pub fn link_lifetimes<'def, 'tcx>(
&'def self,
tcx: &'tcx TypeContext,
) -> LinkedLifetimes<'def, 'tcx> {
match self {
Self::Struct(p) => p.link_lifetimes(tcx),
Self::OutStruct(p) => p.link_lifetimes(tcx),
}
}
}

impl<P: TyPosition> StructPath<P> {
Expand All @@ -149,17 +137,6 @@ impl StructPath {
pub fn resolve<'tcx>(&self, tcx: &'tcx TypeContext) -> &'tcx StructDef {
tcx.resolve_struct(self.tcx_id)
}

/// Get a map of lifetimes used on this path to lifetimes as named in the def site. See [`LinkedLifetimes`]
/// for more information.
pub fn link_lifetimes<'def, 'tcx>(
&'def self,
tcx: &'tcx TypeContext,
) -> LinkedLifetimes<'def, 'tcx> {
let struc = self.resolve(tcx);
let env = &struc.lifetimes;
LinkedLifetimes::new(env, None, &self.lifetimes)
}
}

impl OutStructPath {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: core/src/hir/type_context.rs
expression: output
---
Lowering error: Opaque::use_foo should explicitly include this lifetime bound from param foo: 'y: 'x (comes from source type's 'b: 'a)
Lowering error: Opaque::use_foo should explicitly include this lifetime bound from param foo: 'z: 'y (comes from source type's 'c: 'b)
Lowering error: Opaque::return_foo should explicitly include this lifetime bound from param return type: 'y: 'x (comes from source type's 'b: 'a)
Lowering error: Opaque::return_foo should explicitly include this lifetime bound from param return type: 'z: 'y (comes from source type's 'c: 'b)
Lowering error: Opaque::return_result_foo should explicitly include this lifetime bound from param return type: 'y: 'x (comes from source type's 'b: 'a)
Lowering error: Opaque::return_result_foo should explicitly include this lifetime bound from param return type: 'z: 'y (comes from source type's 'c: 'b)

Loading
Loading