Documentation, refactoring and convenience (#123)

## What Changed?

Moves the annotations into `paralegal-flow`
Provides basic documentation for `paralegal-spdg`
Now requires documentation for `paralegal-spdg` in CI 
Adds convenience functions to `Context`

## Why Does It Need To?

Adds convenience methods to `Context` to improve ergonomics of the
policies.

Adds missing documentation to the spdg crate also now enables the
missing documentation warning, which makes it an error in CI.

The annotations are moved because they aren't in the SPDG anyway and
thus not accessible to consumers in this form.

## 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.

crates/paralegal-flow/Cargo.toml

@@ -40,6 +40,7 @@ num-derive = "0.4"
 num-traits = "0.2"
 petgraph = { workspace = true }
 humantime = "2"
+strum = { version = "0.25", features = ["derive"] }
 
 
 #dot = "0.1"

crates/paralegal-flow/src/ana/mod.rs

@@ -5,6 +5,7 @@
 //! [`analyze`](SPDGGenerator::analyze).
 
 use crate::{
+    ann::{Annotation, MarkerAnnotation},
     desc::*,
     rust::{hir::def, *},
     utils::*,

crates/paralegal-flow/src/ann/mod.rs

@@ -0,0 +1,138 @@
+use serde::{Deserialize, Serialize};
+
+use paralegal_spdg::{rustc_proxies, tiny_bitset_pretty, Identifier, TinyBitSet, TypeId};
+
+pub mod parse;
+
+/// 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_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 {
+    Marker(MarkerAnnotation),
+    OType(#[serde(with = "rustc_proxies::DefId")] TypeId),
+    Exception(ExceptionAnnotation),
+}
+
+impl Annotation {
+    /// If this is an [`Annotation::Marker`], returns the underlying [`MarkerAnnotation`].
+    pub fn as_marker(&self) -> Option<&MarkerAnnotation> {
+        match self {
+            Annotation::Marker(l) => Some(l),
+            _ => None,
+        }
+    }
+
+    /// If this is an [`Annotation::OType`], returns the underlying [`TypeId`].
+    pub fn as_otype(&self) -> Option<TypeId> {
+        match self {
+            Annotation::OType(t) => Some(*t),
+            _ => None,
+        }
+    }
+
+    /// If this is an [`Annotation::Exception`], returns the underlying [`ExceptionAnnotation`].
+    pub fn as_exception(&self) -> Option<&ExceptionAnnotation> {
+        match self {
+            Annotation::Exception(e) => Some(e),
+            _ => None,
+        }
+    }
+}
+
+pub type VerificationHash = u128;
+
+#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Serialize, Deserialize)]
+pub struct ExceptionAnnotation {
+    /// The value of the verification hash we found in the annotation. Is `None`
+    /// if there was no verification hash in the annotation.
+    pub verification_hash: Option<VerificationHash>,
+}
+
+/// A marker annotation and its refinements.
+#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Serialize, Deserialize)]
+pub struct MarkerAnnotation {
+    /// The (unchanged) name of the marker as provided by the user
+    pub marker: Identifier,
+    #[serde(flatten)]
+    pub refinement: MarkerRefinement,
+}
+
+fn const_false() -> bool {
+    false
+}
+
+/// 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)]
+pub struct MarkerRefinement {
+    #[serde(default, with = "tiny_bitset_pretty")]
+    on_argument: TinyBitSet,
+    #[serde(default = "const_false")]
+    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 = "tiny_bitset_pretty")] TinyBitSet),
+    Return,
+}
+
+impl MarkerRefinement {
+    /// The default, empty aggregate refinement `Self { on_argument: vec![],
+    /// on_return: false }`
+    pub fn empty() -> Self {
+        Self {
+            on_argument: Default::default(),
+            on_return: false,
+        }
+    }
+
+    /// Merge the aggregate refinement with another discovered refinement and
+    /// check that they do not overwrite each other.
+    pub fn merge_kind(mut self, k: MarkerRefinementKind) -> Result<Self, String> {
+        match k {
+            MarkerRefinementKind::Argument(a) => {
+                if self.on_argument.is_empty() {
+                    self.on_argument = a;
+                    Ok(self)
+                } else {
+                    Err(format!(
+                        "Double argument annotation {:?} and {a:?}",
+                        self.on_argument
+                    ))
+                }
+            }
+            MarkerRefinementKind::Return => {
+                if !self.on_return {
+                    self.on_return = true;
+                    Ok(self)
+                } else {
+                    Err("Double on-return annotation".to_string())
+                }
+            }
+        }
+    }
+
+    /// 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
+    }
+
+    /// True if this refinement is empty, i.e. the annotation is targeting the
+    /// item itself.
+    pub fn on_self(&self) -> bool {
+        self.on_argument.is_empty() && !self.on_return
+    }
+}

crates/paralegal-flow/src/ann_parse.rs → crates/paralegal-flow/src/ann/parse.rs

@@ -8,9 +8,11 @@
 //! it gives us boundaries for parsers that lets us (re)combine them, but also
 //! that we get features that are annoying to implement (such as backtracking)
 //! for free.
+use super::{
+    ExceptionAnnotation, MarkerAnnotation, MarkerRefinement, MarkerRefinementKind, VerificationHash,
+};
 use crate::{
     consts,
-    desc::{ExceptionAnnotation, MarkerAnnotation, MarkerRefinement, MarkerRefinementKind},
     rust::*,
     utils,
     utils::{write_sep, Print, TinyBitSet},
@@ -250,7 +252,7 @@ pub(crate) fn match_exception(ann: &rustc_ast::AttrArgs) -> ExceptionAnnotation
                         assert_token(TokenKind::Eq),
                     )),
                     lit(token::LitKind::Str, |s| {
-                        crate::desc::VerificationHash::from_str_radix(s, 16)
+                        VerificationHash::from_str_radix(s, 16)
                             .map_err(|e: std::num::ParseIntError| e.to_string())
                     }),
                 ))(i)?;

crates/paralegal-flow/src/lib.rs

@@ -79,7 +79,7 @@ pub use either::Either;
 pub use rustc_span::Symbol;
 
 pub mod ana;
-pub mod ann_parse;
+pub mod ann;
 mod args;
 pub mod dbg;
 mod discover;

crates/paralegal-flow/src/marker_db.rs

@@ -11,9 +11,9 @@
 //! All interactions happen through the central database object: [`MarkerCtx`].
 
 use crate::{
+    ann::{Annotation, MarkerAnnotation},
     args::{Args, MarkerControl},
     consts,
-    desc::{Annotation, MarkerAnnotation},
     hir::def::DefKind,
     mir, ty,
     utils::{
@@ -303,24 +303,22 @@ impl<'tcx> MarkerDatabase<'tcx> {
 
     /// Retrieve and parse the local annotations for this item.
     pub fn retrieve_local_annotations_for(&mut self, def_id: LocalDefId) {
+        use crate::ann::parse::{ann_match_fn, match_exception, otype_ann_match};
+
         let tcx = self.tcx;
         let hir = tcx.hir();
         let id = def_id.force_into_hir_id(tcx);
         let mut sink_matches = vec![];
         for a in hir.attrs(id) {
             if let Some(i) = a.match_get_ref(&consts::MARKER_MARKER) {
-                sink_matches.push(Annotation::Marker(crate::ann_parse::ann_match_fn(i)));
+                sink_matches.push(Annotation::Marker(ann_match_fn(i)));
             } else if let Some(i) = a.match_get_ref(&consts::LABEL_MARKER) {
                 warn!("The `paralegal_flow::label` annotation is deprecated, use `paralegal_flow::marker` instead");
-                sink_matches.push(Annotation::Marker(crate::ann_parse::ann_match_fn(i)))
+                sink_matches.push(Annotation::Marker(ann_match_fn(i)))
             } else if let Some(i) = a.match_get_ref(&consts::OTYPE_MARKER) {
-                sink_matches.extend(
-                    crate::ann_parse::otype_ann_match(i, tcx)
-                        .into_iter()
-                        .map(Annotation::OType),
-                );
+                sink_matches.extend(otype_ann_match(i, tcx).into_iter().map(Annotation::OType));
             } else if let Some(i) = a.match_get_ref(&consts::EXCEPTION_MARKER) {
-                sink_matches.push(Annotation::Exception(crate::ann_parse::match_exception(i)));
+                sink_matches.push(Annotation::Exception(match_exception(i)));
             }
         }
         if !sink_matches.is_empty() {
@@ -332,7 +330,7 @@ impl<'tcx> MarkerDatabase<'tcx> {
     }
 }
 
-type RawExternalMarkers = HashMap<String, Vec<crate::desc::MarkerAnnotation>>;
+type RawExternalMarkers = HashMap<String, Vec<crate::ann::MarkerAnnotation>>;
 
 /// Given the TOML of external annotations we have parsed, resolve the paths
 /// (keys of the map) to [`DefId`]s.

crates/paralegal-policy/src/context.rs

@@ -3,8 +3,9 @@ use std::{io::Write, process::exit, sync::Arc};
 pub use paralegal_spdg::rustc_portable::{DefId, LocalDefId};
 use paralegal_spdg::traverse::{generic_flows_to, EdgeSelection};
 use paralegal_spdg::{
-    CallString, DisplayNode, Endpoint, GlobalNode, HashMap, Identifier, IntoIterGlobalNodes,
-    Node as SPDGNode, NodeCluster, ProgramDescription, SPDGImpl, TypeId, SPDG,
+    CallString, DisplayNode, Endpoint, GlobalNode, HashMap, Identifier, InstructionInfo,
+    IntoIterGlobalNodes, Node as SPDGNode, NodeCluster, NodeInfo, ProgramDescription, SPDGImpl,
+    TypeId, SPDG,
 };
 
 use anyhow::{anyhow, bail, ensure, Result};
@@ -71,7 +72,7 @@ fn bfs_iter<
     }
     let bfs = Bfs { stack, discovered };
     let walker_iter = Walker::iter(bfs, g);
-    walker_iter.map(move |inner| GlobalNode::unsafe_new(controller_id, inner.index()))
+    walker_iter.map(move |inner| GlobalNode::from_local_node(controller_id, inner))
 }
 
 /// Interface for defining policies.
@@ -587,6 +588,33 @@ impl Context {
         )
     }
 
+    /// Retrieve metadata about a node.
+    pub fn node_info(&self, node: GlobalNode) -> &NodeInfo {
+        self.desc.controllers[&node.controller_id()].node_info(node.local_node())
+    }
+
+    /// Retrieve metadata about the instruction executed by a specific node.
+    pub fn instruction_at_node(&self, node: GlobalNode) -> &InstructionInfo {
+        let node_info = self.node_info(node);
+        &self.desc.instruction_info[&node_info.at.leaf()]
+    }
+
+    /// Return the immediate successors of this node
+    pub fn successors(&self, node: GlobalNode) -> impl Iterator<Item = GlobalNode> + '_ {
+        self.desc.controllers[&node.controller_id()]
+            .graph
+            .neighbors(node.local_node())
+            .map(move |n| GlobalNode::from_local_node(node.controller_id(), n))
+    }
+
+    /// Return the immediate predecessors of this node
+    pub fn predecessors(&self, node: GlobalNode) -> impl Iterator<Item = GlobalNode> + '_ {
+        self.desc.controllers[&node.controller_id()]
+            .graph
+            .neighbors_directed(node.local_node(), petgraph::Direction::Incoming)
+            .map(move |n| GlobalNode::from_local_node(node.controller_id(), n))
+    }
+
     #[cfg(test)]
     pub fn nth_successors(
         &self,

crates/paralegal-policy/src/lib.rs

@@ -53,7 +53,9 @@ extern crate core;
 
 use anyhow::{ensure, Result};
 pub use paralegal_spdg;
-use paralegal_spdg::ProgramDescription;
+pub use paralegal_spdg::{
+    traverse::EdgeSelection, GlobalNode, IntoIterGlobalNodes, ProgramDescription,
+};
 use std::{
     fs::File,
     path::{Path, PathBuf},

crates/paralegal-spdg/src/dot.rs

@@ -1,3 +1,5 @@
+//! Display SPDGs as dot graphs
+
 use crate::{GlobalEdge, InstructionInfo, Node, ProgramDescription};
 use dot::{CompassPoint, Edges, Id, LabelText, Nodes};
 use flowistry_pdg::rustc_portable::LocalDefId;
@@ -186,10 +188,12 @@ impl<'a, 'd> dot::Labeller<'a, CallString, GlobalEdge> for DotPrintableProgramDe
     }
 }
 
+/// Dump all SPDGs in a single dot expression
 pub fn dump<W: std::io::Write>(spdg: &ProgramDescription, out: W) -> std::io::Result<()> {
     dump_for_selection(spdg, out, |_| true)
 }
 
+/// Dump the SPDG for one select controller in dot format
 pub fn dump_for_controller(
     spdg: &ProgramDescription,
     out: impl std::io::Write,
@@ -210,6 +214,7 @@ pub fn dump_for_controller(
     Ok(())
 }
 
+/// Dump a selection of controllers into a dot expression.
 pub fn dump_for_selection(
     spdg: &ProgramDescription,
     mut out: impl std::io::Write,