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

Various changes, mostly for error handling #9

Merged
merged 9 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,5 @@ jobs:
- uses: dtolnay/rust-toolchain@master
with:
toolchain: nightly-2024-09-15
override: false
- name: Run tests
run: cargo test --test run_all_tests
16 changes: 15 additions & 1 deletion Cargo.lock

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

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ edition = "2021"
build = "build.rs"

[dependencies]
shell-escape = "0.1.5"
rustversion = "1.0"
itertools = "0.12.0"
derive_more = "0.99"
dot = "0.1"
Expand Down
133 changes: 44 additions & 89 deletions src/borrows/borrow_pcg_expansion.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};

use itertools::Itertools;
use serde_json::json;
Expand All @@ -12,10 +12,9 @@ use crate::{
ty,
},
span::Symbol,
target::abi::FieldIdx,
target::abi::VariantIdx,
target::abi::{FieldIdx, VariantIdx},
},
utils::{CorrectedPlace, HasPlace, Place, PlaceRepacker},
utils::{ConstantIndex, CorrectedPlace, HasPlace, Place, PlaceRepacker},
};

use super::{
Expand Down Expand Up @@ -60,101 +59,59 @@ pub(crate) enum BorrowExpansion<'tcx> {
/// See [`PlaceElem::Index`]
Index(Local),
/// See [`PlaceElem::ConstantIndex`]
ConstantIndex {
offset: u64,
min_length: u64,
from_end: bool,
},
ConstantIndices(BTreeSet<ConstantIndex>),
/// See [`PlaceElem::Subslice`]
Subslice { from: u64, to: u64, from_end: bool },
}

impl<'tcx> BorrowExpansion<'tcx> {
pub(super) fn from_places(
base_place: Place<'tcx>,
places: Vec<Place<'tcx>>,
repacker: PlaceRepacker<'_, 'tcx>,
) -> Self {
let place_ty = base_place.ty(repacker);

// The maximum number of places expected in the expansion, according to
// the type
let max_places = match place_ty.ty.kind() {
ty::TyKind::Adt(adt_def, _) => {
if adt_def.is_box() {
1
} else if adt_def.is_enum() {
place_ty
.variant_index
.map(|variant_idx| adt_def.variant(variant_idx).fields.len())
.unwrap_or(1)
} else {
adt_def.non_enum_variant().fields.len()
}
}
ty::TyKind::Tuple(tys) => tys.len(),
ty::TyKind::Closure(_, substs) => substs.as_closure().upvar_tys().len(),
_ => 1,
};

assert!(
places.len() <= max_places,
"Based on the type {:?}, we expected no more than {} places, but got {} ({:?})",
place_ty,
max_places,
places.len(),
places
);

if max_places > 1 {
// This is definitely something with multiple fields.
let fields = places
.iter()
.map(
|p| match CorrectedPlace::new(*p, repacker).last_projection() {
Some(elem) => match *elem {
PlaceElem::Field(field_idx, ty) => (field_idx, ty),
other => panic!("Expected a field projection, got {:?}", other),
},
_ => panic!("Expected a field projection"),
},
)
.collect();
BorrowExpansion::Fields(fields)
} else {
match CorrectedPlace::new(places[0], repacker).last_projection() {
Some(elem) => match *elem {
pub(super) fn from_places(places: Vec<Place<'tcx>>, repacker: PlaceRepacker<'_, 'tcx>) -> Self {
let mut fields = BTreeMap::new();
let mut constant_indices = BTreeSet::new();

for place in places {
let corrected_place = CorrectedPlace::new(place, repacker);
let last_projection = corrected_place.last_projection();
if let Some(elem) = last_projection {
match *elem {
PlaceElem::Field(field_idx, ty) => {
return BorrowExpansion::Fields([(field_idx, ty)].into_iter().collect());
}
PlaceElem::Deref => {
return BorrowExpansion::Deref;
}
PlaceElem::Downcast(symbol, variant_idx) => {
return BorrowExpansion::Downcast(symbol, variant_idx);
}
PlaceElem::Index(idx) => {
return BorrowExpansion::Index(idx);
fields.insert(field_idx, ty);
}
PlaceElem::ConstantIndex {
offset,
min_length,
from_end,
} => {
return BorrowExpansion::ConstantIndex {
constant_indices.insert(ConstantIndex {
offset,
min_length,
from_end,
};
});
}
PlaceElem::Deref => return BorrowExpansion::Deref,
PlaceElem::Downcast(symbol, variant_idx) => {
return BorrowExpansion::Downcast(symbol, variant_idx);
}
PlaceElem::Index(idx) => {
return BorrowExpansion::Index(idx);
}
PlaceElem::Subslice { from, to, from_end } => {
return BorrowExpansion::Subslice { from, to, from_end };
}
other => todo!("{other:?} for type {:?}", place_ty.ty),
},
_ => unreachable!(),
PlaceElem::OpaqueCast(_) => todo!(),
PlaceElem::Subtype(_) => todo!(),
}
}
}

if !fields.is_empty() {
assert!(constant_indices.is_empty());
BorrowExpansion::Fields(fields)
} else if !constant_indices.is_empty() {
BorrowExpansion::ConstantIndices(constant_indices)
} else {
unreachable!()
}
}

pub(super) fn elems(&self) -> Vec<PlaceElem<'tcx>> {
Expand All @@ -169,17 +126,15 @@ impl<'tcx> BorrowExpansion<'tcx> {
vec![PlaceElem::Downcast(*symbol, *variant_idx)]
}
BorrowExpansion::Index(idx) => vec![PlaceElem::Index(*idx)],
BorrowExpansion::ConstantIndex {
offset,
min_length,
from_end,
} => {
vec![PlaceElem::ConstantIndex {
offset: *offset,
min_length: *min_length,
from_end: *from_end,
}]
}
BorrowExpansion::ConstantIndices(constant_indices) => constant_indices
.iter()
.sorted_by_key(|a| a.offset)
.map(|c| PlaceElem::ConstantIndex {
offset: c.offset,
min_length: c.min_length,
from_end: c.from_end,
})
.collect(),
BorrowExpansion::Subslice { from, to, from_end } => {
vec![PlaceElem::Subslice {
from: *from,
Expand Down
3 changes: 2 additions & 1 deletion src/borrows/borrows_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use super::{
latest::Latest,
path_condition::{PathCondition, PathConditions},
region_abstraction::AbstractionEdge,
region_projection_member::RegionProjectionMember,
region_projection_member::{RegionProjectionMember, RegionProjectionMemberKind},
};

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -471,6 +471,7 @@ impl<'tcx> BorrowsGraph<'tcx> {
.map(|rp| rp.try_into().unwrap())
.collect::<Vec<_>>()
.into(),
RegionProjectionMemberKind::Todo,
));
let inserted =
self.insert(BorrowPCGEdge::new(new_edge_kind, edge.conditions.clone()));
Expand Down
18 changes: 14 additions & 4 deletions src/borrows/borrows_state.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use tracing::instrument;

use crate::{
borrows::edge_data::EdgeData,
borrows::{edge_data::EdgeData, region_projection_member::RegionProjectionMemberKind},
rustc_interface::{
ast::Mutability,
data_structures::fx::FxHashSet,
Expand Down Expand Up @@ -602,6 +602,7 @@ impl<'tcx> BorrowsState<'tcx> {
RegionProjectionMember::new(
Coupled::singleton(reborrow.blocked_place.into()),
Coupled::singleton(ra.into()),
RegionProjectionMemberKind::Todo,
),
PathConditions::new(location.block),
"Ensure Expansion To",
Expand Down Expand Up @@ -688,6 +689,7 @@ impl<'tcx> BorrowsState<'tcx> {
RegionProjection::new((*region).into(), base).into(),
),
Coupled::singleton(target.into()),
RegionProjectionMemberKind::Todo,
),
PathConditions::new(location.block),
"Ensure Deref Expansion To At Least",
Expand All @@ -711,7 +713,7 @@ impl<'tcx> BorrowsState<'tcx> {
BorrowPCGAction::insert_borrow_pcg_expansion(
BorrowPCGExpansion::from_borrowed_base(
rp.into(),
BorrowExpansion::from_places(base, dest_places, repacker),
BorrowExpansion::from_places(dest_places, repacker),
repacker,
),
location,
Expand All @@ -727,7 +729,7 @@ impl<'tcx> BorrowsState<'tcx> {
let action = BorrowPCGAction::insert_borrow_pcg_expansion(
BorrowPCGExpansion::new(
origin_place.into(),
BorrowExpansion::from_places(base, expansion, repacker),
BorrowExpansion::from_places(expansion, repacker),
repacker,
),
location,
Expand Down Expand Up @@ -859,7 +861,14 @@ impl<'tcx> BorrowsState<'tcx> {
region: ty::Region<'tcx>,
repacker: PlaceRepacker<'_, 'tcx>,
) -> ExecutedActions<'tcx> {
assert!(assigned_place.ty(repacker).ty.ref_mutability().is_some());
assert!(
assigned_place.ty(repacker).ty.ref_mutability().is_some(),
"{:?}:{:?} Assigned place {:?} is not a reference. Ty: {:?}",
repacker.body().source.def_id(),
location,
assigned_place,
assigned_place.ty(repacker).ty
);
let mut actions = ExecutedActions::new();
let (blocked_cap, assigned_cap) = match mutability {
Mutability::Not => (CapabilityKind::Read, CapabilityKind::Read),
Expand All @@ -879,6 +888,7 @@ impl<'tcx> BorrowsState<'tcx> {
RegionProjectionMember::new(
Coupled::singleton(rp.into()),
Coupled::singleton(assigned_place.into()),
RegionProjectionMemberKind::Todo,
),
PathConditions::new(location.block),
"Add Borrow",
Expand Down
Loading