Skip to content

Commit

Permalink
Rename label to marker, adjust documentation (#40)
Browse files Browse the repository at this point in the history 
## What Changed?

Renames the remaining occurrences of `label` in the graph crate to
`marker`. Also fixes some of the documentation there.

Also removed the unused `merge` method

## Why Does It Need To?

Having both the `marker` and `label` name float around is confusing.

## 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
- [x] Documentation in Notion has been updated
- [x] Tests for new behaviors are provided
  - [x] 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
JustusAdam committed Sep 16, 2023
1 parent e5ed259 commit 1afc6b6
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 95 deletions.
29 changes: 29 additions & 0 deletions Cargo.lock
View file

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

8 changes: 4 additions & 4 deletions crates/dfcheck/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl Context {
.iter()
.flat_map(|(id, (annots, _))| {
annots.iter().filter_map(move |annot| match annot {
Annotation::Label(MarkerAnnotation { marker, refinement }) => {
Annotation::Marker(MarkerAnnotation { marker, refinement }) => {
Some((*marker, (*id, refinement.clone())))
}
_ => None,
Expand Down Expand Up @@ -305,7 +305,7 @@ fn test_happens_before() -> Result<()> {
desc.annotations
.get(&function.function)
.map_or(false, |(anns, _)| {
anns.iter().filter_map(Annotation::as_label_ann).any(|ann| {
anns.iter().filter_map(Annotation::as_marker).any(|ann| {
ann.marker == marker
&& (ann
.refinement
Expand Down Expand Up @@ -340,7 +340,7 @@ fn test_happens_before() -> Result<()> {
DataSource::Argument(arg) => desc.annotations[&ctrl_name]
.0
.iter()
.filter_map(Annotation::as_label_ann)
.filter_map(Annotation::as_marker)
.any(|ann| {
ann.marker == marker
&& (ann.refinement.on_self()
Expand All @@ -353,7 +353,7 @@ fn test_happens_before() -> Result<()> {
DataSource::FunctionCall(cs) => desc.annotations[&cs.function]
.0
.iter()
.filter_map(Annotation::as_label_ann)
.filter_map(Annotation::as_marker)
.any(|ann| {
ann.marker == marker
&& (ann.refinement.on_self() || ann.refinement.on_return())
Expand Down
1 change: 1 addition & 0 deletions crates/dfgraph/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ log = "0.4"
internment = { version = "0.7.1", features = ["serde"] }
indexical = { workspace = true }
itertools = "0.11.0"
strum = { version = "0.25", features = ["derive"] }
65 changes: 23 additions & 42 deletions crates/dfgraph/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ pub(crate) mod rustc {
pub use middle::mir;
}

extern crate strum;

pub mod global_location;
#[cfg(feature = "rustc")]
mod rustc_impls;
Expand All @@ -29,7 +31,6 @@ use global_location::GlobalLocation;
use indexical::define_index_type;
use internment::Intern;
use itertools::Itertools;
use log::warn;
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use std::{borrow::Cow, fmt, hash::Hash, iter};

Expand All @@ -43,39 +44,35 @@ pub type Function = Identifier;
/// Types of annotations we support.
///
/// Usually you'd expect one of those annotation types in any given situation.
/// For convenience the match methods [`Self::as_label_ann`],
/// [`Self::as_otype_ann`] and [`Self::as_exception_annotation`] are provided. These are particularly useful in conjunction with e.g. [`Iterator::filter_map`]
#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Deserialize, Serialize)]
/// For convenience the match methods [`Self::as_marker`], [`Self::as_otype`]
/// and [`Self::as_exception`] are provided. These are particularly useful in
/// conjunction with e.g. [`Iterator::filter_map`]
#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Deserialize, Serialize, strum::EnumIs)]
pub enum Annotation {
Label(MarkerAnnotation),
Marker(MarkerAnnotation),
OType(Vec<TypeDescriptor>),
Exception(ExceptionAnnotation),
}

impl Annotation {
/// If this is an [`Annotation::Label`], returns the underlying [`MarkerAnnotation`].
pub fn as_label_ann(&self) -> Option<&MarkerAnnotation> {
/// If this is an [`Annotation::Marker`], returns the underlying [`MarkerAnnotation`].
pub fn as_marker(&self) -> Option<&MarkerAnnotation> {
match self {
Annotation::Label(l) => Some(l),
Annotation::Marker(l) => Some(l),
_ => None,
}
}

/// Returns true if this is an [`Annotation::Label`].
pub fn is_label_ann(&self) -> bool {
matches!(self, Annotation::Label(_))
}

/// If this is an [`Annotation::OType`], returns the underlying [`TypeDescriptor`].
pub fn as_otype_ann(&self) -> Option<&[TypeDescriptor]> {
pub fn as_otype(&self) -> Option<&[TypeDescriptor]> {
match self {
Annotation::OType(t) => Some(t),
_ => None,
}
}

/// If this is an [`Annotation::Exception`], returns the underlying [`ExceptionAnnotation`].
pub fn as_exception_annotation(&self) -> Option<&ExceptionAnnotation> {
pub fn as_exception(&self) -> Option<&ExceptionAnnotation> {
match self {
Annotation::Exception(e) => Some(e),
_ => None,
Expand All @@ -92,10 +89,10 @@ pub struct ExceptionAnnotation {
pub verification_hash: Option<VerificationHash>,
}

/// A label annotation and its refinements.
/// A marker annotation and its refinements.
#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Serialize, Deserialize)]
pub struct MarkerAnnotation {
/// The (unchanged) name of the label as provided by the user
/// The (unchanged) name of the marker as provided by the user
pub marker: Identifier,
#[serde(flatten)]
pub refinement: MarkerRefinement,
Expand All @@ -105,7 +102,7 @@ fn const_false() -> bool {
false
}

/// Refinements in the label targeting. The default (no refinement provided) is
/// Refinements in the marker targeting. The default (no refinement provided) is
/// `on_argument == vec![]` and `on_return == false`, which is also what is
/// returned from [`Self::empty`].
#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Deserialize, Serialize)]
Expand All @@ -116,6 +113,8 @@ pub struct MarkerRefinement {
on_return: bool,
}

/// Disaggregated version of [`MarkerRefinement`]. Can be added to an existing
/// refinement [`MarkerRefinement::merge_kind`].
#[derive(Clone, Deserialize, Serialize)]
pub enum MarkerRefinementKind {
Argument(#[serde(with = "crate::tiny_bitset::pretty")] TinyBitSet),
Expand Down Expand Up @@ -158,10 +157,12 @@ impl MarkerRefinement {
}
}

/// Get the refinements on arguments
pub fn on_argument(&self) -> TinyBitSet {
self.on_argument
}

/// Is this refinement targeting the return value?
pub fn on_return(&self) -> bool {
self.on_return
}
Expand All @@ -173,41 +174,21 @@ impl MarkerRefinement {
}
}

#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Serialize, Deserialize)]
#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Serialize, Deserialize, strum::EnumIs)]
pub enum ObjectType {
Function(usize),
Type,
Other,
}

impl ObjectType {
pub fn is_function(&self) -> Option<usize> {
/// If this is [`Self::Function`], then return the payload.
pub fn as_function(&self) -> Option<usize> {
match self {
ObjectType::Function(f) => Some(*f),
_ => None,
}
}
pub fn merge(&mut self, other: &Self) {
if self != other {
warn!(
"Merging two different object types {:?} and {:?}!",
self, other
);
match (self, other) {
(ObjectType::Function(ref mut i), ObjectType::Function(j)) => {
if j > i {
*i = *j
}
}
(slf, other) => {
panic!("Cannot merge two different object types {slf:?} and {other:?}")
}
}
}
}
pub fn is_type(&self) -> bool {
matches!(self, ObjectType::Type)
}
}

pub type AnnotationMap = HashMap<Identifier, (Vec<Annotation>, ObjectType)>;
Expand Down Expand Up @@ -305,7 +286,7 @@ impl ProgramDescription {
.chain(
self.annotations
.iter()
.filter(|f| f.1 .1.is_function().is_some())
.filter(|f| f.1 .1.as_function().is_some())
.map(|f| f.0),
)
.collect()
Expand Down
2 changes: 1 addition & 1 deletion crates/dfpp/src/ana/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ impl<'tcx> CollectingVisitor<'tcx> {
.chain(self.marker_ctx.external_annotations().iter().map(|(did, anns)| {
(
*did,
anns.iter().cloned().map(Annotation::Label).collect(),
anns.iter().cloned().map(Annotation::Marker).collect(),
)
}))
.map(|(did, anns)| {
Expand Down
87 changes: 44 additions & 43 deletions crates/dfpp/src/frg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ impl ProgramDescriptionExt for ProgramDescription {
self.annotations
.values()
.flat_map(|v| v.0.iter())
.filter_map(Annotation::as_label_ann)
.filter_map(Annotation::as_marker)
.map(|a| a.marker)
.chain(std::iter::once(Identifier::new_intern(
name::EXCEPTIONS_LABEL,
Expand Down Expand Up @@ -678,59 +678,60 @@ impl ProgramDescriptionExt for ProgramDescription {
// Part of why we choose this behavior is because there is no
// call-site-independent representation for arguments, so the
// label has to be attached to the call site argument.
anns.iter()
.filter_map(Annotation::as_label_ann)
.map(move |a| {
(
if a.refinement.on_return() {
Some(self.all_sources_with_ctrl().into_iter().filter(|(_, s)| {
s.as_function_call().map_or(false, |c| &c.function == id)
}))
} else {
None
}
.into_iter()
.flatten()
.map(|(ctrl, ds)| data_source_as_forge(ds, alloc, ctrl))
.chain(
self.all_sinks()
.into_iter()
.filter(|s| {
matches!(
s,
DataSink::Argument{function, arg_slot} if
&function.function == id
&& a.refinement
.on_argument()
.is_set(*arg_slot as u32)
)
})
.map(|s| s.build_forge(alloc)),
)
.chain([id.build_forge(alloc)])
.chain(a.refinement.on_argument().into_iter_set_in_domain().map(
|slot| {
anns.iter().filter_map(Annotation::as_marker).map(move |a| {
(
if a.refinement.on_return() {
Some(self.all_sources_with_ctrl().into_iter().filter(|(_, s)| {
s.as_function_call().map_or(false, |c| &c.function == id)
}))
} else {
None
}
.into_iter()
.flatten()
.map(|(ctrl, ds)| data_source_as_forge(ds, alloc, ctrl))
.chain(
self.all_sinks()
.into_iter()
.filter(|s| {
matches!(
s,
DataSink::Argument{function, arg_slot} if
&function.function == id
&& a.refinement
.on_argument()
.is_set(*arg_slot as u32)
)
})
.map(|s| s.build_forge(alloc)),
)
.chain([id.build_forge(alloc)])
.chain(
a.refinement
.on_argument()
.into_iter_set_in_domain()
.map(|slot| {
FormalParameter {
function: *id,
position: slot as u16,
}
.build_forge(alloc)
},
))
// This is necessary because otherwise captured variables escape
.collect::<Vec<_>>()
.into_iter(),
std::iter::once(a.marker.build_forge(alloc)),
}),
)
})
// This is necessary because otherwise captured variables escape
.collect::<Vec<_>>()
.into_iter(),
std::iter::once(a.marker.build_forge(alloc)),
)
})
}))
.append(" +")
.append(alloc.hardline())
.append(
alloc.forge_relation(self.annotations.iter().map(|(id, (anns, _))| {
(
anns.iter()
.filter_map(Annotation::as_exception_annotation)
.filter_map(Annotation::as_exception)
.next()
.map(|_| id.build_forge(alloc))
.into_iter(),
Expand Down Expand Up @@ -783,7 +784,7 @@ impl ProgramDescriptionExt for ProgramDescription {
(
std::iter::once(o.build_forge(alloc)),
anns.iter()
.filter_map(Annotation::as_otype_ann)
.filter_map(Annotation::as_otype)
.flat_map(|v| v.iter())
.map(|t| t.build_forge(alloc)),
)
Expand Down Expand Up @@ -1144,7 +1145,7 @@ impl ProgramDescriptionExt for ProgramDescription {
alloc.forge_relation_with_arity(3,
self.annotations.iter()
.flat_map(|(ident, (anns, _))|
anns.iter().filter_map(Annotation::as_label_ann)
anns.iter().filter_map(Annotation::as_marker)
.flat_map(|label|
label.refinement.on_argument().into_iter_set_in_domain().map(|i|
(
Expand Down
Loading

0 comments on commit 1afc6b6

Please sign in to comment.