tamer: asg::Asg::set_fragment: {ObjectRef=>SymbolId}

In the actual implementation (outside of tests), this is always looking up
before adding the symbol.  This will simplify the API, while still retaining
errors, since the identifier will fail the state transition if the
identifier did not exist before attempting to set a fragment.  So while this
is slower in microbenchmarks, this has no effect on real-world performance.

Further, I'm refactoring toward a streaming ASG aggregation, which is a lot
easier if we do not need to perform lookups in a separate step from the
ASG's primitives.

DEV-11864
main
Mike Gerwitz 2022-05-16 10:53:07 -04:00
parent c49d87976d
commit 34eb994a0d
5 changed files with 106 additions and 159 deletions

View File

@ -117,19 +117,15 @@ mod base {
let mut sut = Sut::new();
let xs = interned_n(1_000);
let orefs = xs
.iter()
.map(|sym| {
sut.declare(*sym, IdentKind::Meta, Source::default())
.unwrap()
})
.collect::<Vec<_>>();
xs.iter().for_each(|sym| {
sut.declare(*sym, IdentKind::Meta, Source::default())
.unwrap();
});
// Bench only the resolution, not initial declare.
bench.iter(|| {
orefs
.iter()
.map(|oref| sut.set_fragment(*oref, "".into())) // see N.B.
xs.iter()
.map(|sym| sut.set_fragment(*sym, "".into())) // see N.B.
.for_each(drop);
});
}

View File

@ -291,10 +291,10 @@ impl Asg {
/// see [`IdentObject::set_fragment`].
pub fn set_fragment(
&mut self,
identi: ObjectRef,
name: SymbolId,
text: FragmentText,
) -> AsgResult<ObjectRef> {
self.with_ident(identi, |obj| obj.set_fragment(text))
self.with_ident_lookup(name, |obj| obj.set_fragment(text))
}
/// Retrieve an object from the graph by [`ObjectRef`].
@ -578,7 +578,7 @@ mod test {
let node = sut.declare(sym, IdentKind::Meta, src.clone())?;
let fragment = "a fragment".intern();
let node_with_frag = sut.set_fragment(node, fragment)?;
let node_with_frag = sut.set_fragment(sym, fragment)?;
// Attaching a fragment should _replace_ the node, not create a
// new one
@ -611,10 +611,10 @@ mod test {
let node = sut.declare(sym, IdentKind::Meta, src.clone())?;
// The first set will succeed.
sut.set_fragment(node, "".into())?;
sut.set_fragment(sym, "".into())?;
// This will fail.
let result = sut.set_fragment(node, "".into());
let result = sut.set_fragment(sym, "".into());
// The node should have been restored.
let obj = sut.get(node).unwrap();

View File

@ -157,24 +157,25 @@
//! ```
//! # use tamer::global;
//! # use tamer::asg::{DefaultAsg, IdentKind, IdentObject, FragmentText, Source};
//! # use tamer::sym::{Interner, DefaultProgInterner};
//! # use tamer::sym::{Interner, GlobalSymbolIntern};
//! #
//! # fn main() -> Result<(), Box<dyn std::error::Error>> {
//! # let mut asg = DefaultAsg::with_capacity(
//! # 1024,
//! # 1024,
//! # );
//! # let interner = DefaultProgInterner::new();
//! #
//! let sym = "ident".intern();
//!
//! // Fragments can be attached to resolved identifiers.
//! let ident = asg.declare(
//! interner.intern("ident"), IdentKind::Meta, Source::default()
//! sym, IdentKind::Meta, Source::default()
//! )?;
//! asg.set_fragment(ident, FragmentText::from("test fragment"))?;
//! asg.set_fragment(sym, FragmentText::from("test fragment"))?;
//!
//! assert_eq!(
//! Some(&IdentObject::IdentFragment(
//! interner.intern("ident"),
//! sym,
//! IdentKind::Meta,
//! Source::default(),
//! FragmentText::from("test fragment"),
@ -183,7 +184,7 @@
//! );
//!
//! // But overwriting will fail
//! let bad = asg.set_fragment(ident, FragmentText::from("overwrite"));
//! let bad = asg.set_fragment(sym, FragmentText::from("overwrite"));
//! assert!(bad.is_err());
//! #
//! # Ok(()) // main

View File

@ -188,37 +188,33 @@ mod test {
let text = "dummy fragment".intern();
asg.declare(
st::L_MAP_UUUHEAD.into(),
IdentKind::MapHead,
Default::default(),
)
.and_then(|id| asg.set_fragment(id, text))
.unwrap();
{
let sym = st::L_MAP_UUUHEAD.into();
asg.declare(sym, IdentKind::MapHead, Default::default())
.unwrap();
asg.set_fragment(sym, text).unwrap();
}
asg.declare(
st::L_MAP_UUUTAIL.into(),
IdentKind::MapTail,
Default::default(),
)
.and_then(|id| asg.set_fragment(id, text))
.unwrap();
{
let sym = st::L_MAP_UUUTAIL.into();
asg.declare(sym, IdentKind::MapTail, Default::default())
.unwrap();
asg.set_fragment(sym, text).unwrap();
}
asg.declare(
st::L_RETMAP_UUUHEAD.into(),
IdentKind::RetMapHead,
Default::default(),
)
.and_then(|id| asg.set_fragment(id, text))
.unwrap();
{
let sym = st::L_RETMAP_UUUHEAD.into();
asg.declare(sym, IdentKind::RetMapHead, Default::default())
.unwrap();
asg.set_fragment(sym, text).unwrap();
}
asg.declare(
st::L_RETMAP_UUUTAIL.into(),
IdentKind::RetMapTail,
Default::default(),
)
.and_then(|id| asg.set_fragment(id, text))
.unwrap();
{
let sym = st::L_RETMAP_UUUTAIL.into();
asg.declare(sym, IdentKind::RetMapTail, Default::default())
.unwrap();
asg.set_fragment(sym, text).unwrap();
}
asg
}
@ -325,8 +321,7 @@ mod test {
)
.unwrap();
asg.set_fragment(sym_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(sym, FragmentText::from("foo")).unwrap();
let (_, _) = asg.add_dep_lookup(sym, dep);
@ -392,10 +387,8 @@ mod test {
)
.unwrap();
asg.set_fragment(sym_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(dep_node, FragmentText::from("bar"))
.unwrap();
asg.set_fragment(sym, FragmentText::from("foo")).unwrap();
asg.set_fragment(dep, FragmentText::from("bar")).unwrap();
let (_, _) = asg.add_dep_lookup(sym, dep);
let (_, _) = asg.add_dep_lookup(dep, sym);
@ -466,14 +459,10 @@ mod test {
)
.unwrap();
asg.set_fragment(sym_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(sym2_node, FragmentText::from("bar"))
.unwrap();
asg.set_fragment(dep_node, FragmentText::from("baz"))
.unwrap();
asg.set_fragment(dep2_node, FragmentText::from("huh"))
.unwrap();
asg.set_fragment(sym, FragmentText::from("foo")).unwrap();
asg.set_fragment(sym2, FragmentText::from("bar")).unwrap();
asg.set_fragment(dep, FragmentText::from("baz")).unwrap();
asg.set_fragment(dep2, FragmentText::from("huh")).unwrap();
let (_, _) = asg.add_dep_lookup(sym, dep);
let (_, _) = asg.add_dep_lookup(dep, sym);
@ -513,21 +502,18 @@ mod test {
)
.unwrap();
let dep_node = asg
.declare(
dep,
IdentKind::Tpl,
Source {
virtual_: true,
..Default::default()
},
)
.unwrap();
asg.declare(
dep,
IdentKind::Tpl,
Source {
virtual_: true,
..Default::default()
},
)
.unwrap();
asg.set_fragment(sym_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(dep_node, FragmentText::from("bar"))
.unwrap();
asg.set_fragment(sym, FragmentText::from("foo")).unwrap();
asg.set_fragment(dep, FragmentText::from("bar")).unwrap();
let (_, _) = asg.add_dep_lookup(sym, dep);
let (_, _) = asg.add_dep_lookup(sym, dep);
@ -583,12 +569,9 @@ mod test {
)
.unwrap();
asg.set_fragment(sym1_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(sym2_node, FragmentText::from("bar"))
.unwrap();
asg.set_fragment(sym3_node, FragmentText::from("baz"))
.unwrap();
asg.set_fragment(sym1, FragmentText::from("foo")).unwrap();
asg.set_fragment(sym2, FragmentText::from("bar")).unwrap();
asg.set_fragment(sym3, FragmentText::from("baz")).unwrap();
let (_, _) = asg.add_dep_lookup(sym1, sym2);
let (_, _) = asg.add_dep_lookup(sym2, sym3);
@ -649,12 +632,9 @@ mod test {
)
.unwrap();
asg.set_fragment(sym1_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(sym2_node, FragmentText::from("bar"))
.unwrap();
asg.set_fragment(sym3_node, FragmentText::from("baz"))
.unwrap();
asg.set_fragment(sym1, FragmentText::from("foo")).unwrap();
asg.set_fragment(sym2, FragmentText::from("bar")).unwrap();
asg.set_fragment(sym3, FragmentText::from("baz")).unwrap();
let (_, _) = asg.add_dep_lookup(sym1, sym2);
let (_, _) = asg.add_dep_lookup(sym2, sym3);
@ -714,12 +694,9 @@ mod test {
)
.unwrap();
asg.set_fragment(sym1_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(sym2_node, FragmentText::from("bar"))
.unwrap();
asg.set_fragment(sym3_node, FragmentText::from("baz"))
.unwrap();
asg.set_fragment(sym1, FragmentText::from("foo")).unwrap();
asg.set_fragment(sym2, FragmentText::from("bar")).unwrap();
asg.set_fragment(sym3, FragmentText::from("baz")).unwrap();
let (_, _) = asg.add_dep_lookup(sym1, sym2);
let (_, _) = asg.add_dep_lookup(sym2, sym3);
@ -756,21 +733,18 @@ mod test {
)
.unwrap();
let dep_node = asg
.declare(
dep,
IdentKind::Func(Dim::default(), SymDtype::Empty),
Source {
virtual_: true,
..Default::default()
},
)
.unwrap();
asg.declare(
dep,
IdentKind::Func(Dim::default(), SymDtype::Empty),
Source {
virtual_: true,
..Default::default()
},
)
.unwrap();
asg.set_fragment(sym_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(dep_node, FragmentText::from("bar"))
.unwrap();
asg.set_fragment(sym, FragmentText::from("foo")).unwrap();
asg.set_fragment(dep, FragmentText::from("bar")).unwrap();
let (_, _) = asg.add_dep_lookup(sym, dep);
let (_, _) = asg.add_dep_lookup(dep, sym);
@ -826,12 +800,9 @@ mod test {
)
.unwrap();
asg.set_fragment(sym1_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(sym2_node, FragmentText::from("bar"))
.unwrap();
asg.set_fragment(sym3_node, FragmentText::from("baz"))
.unwrap();
asg.set_fragment(sym1, FragmentText::from("foo")).unwrap();
asg.set_fragment(sym2, FragmentText::from("bar")).unwrap();
asg.set_fragment(sym3, FragmentText::from("baz")).unwrap();
let (_, _) = asg.add_dep_lookup(sym1, sym2);
let (_, _) = asg.add_dep_lookup(sym2, sym3);
@ -869,33 +840,29 @@ mod test {
)
.unwrap();
let dep_node = asg
.declare(
dep,
IdentKind::Tpl,
Source {
virtual_: true,
..Default::default()
},
)
.unwrap();
asg.declare(
dep,
IdentKind::Tpl,
Source {
virtual_: true,
..Default::default()
},
)
.unwrap();
let ignored_node = asg
.declare(
ignored,
IdentKind::Tpl,
Source {
virtual_: true,
..Default::default()
},
)
.unwrap();
asg.declare(
ignored,
IdentKind::Tpl,
Source {
virtual_: true,
..Default::default()
},
)
.unwrap();
asg.set_fragment(sym_node, FragmentText::from("foo"))
.unwrap();
asg.set_fragment(dep_node, FragmentText::from("bar"))
.unwrap();
asg.set_fragment(ignored_node, FragmentText::from("baz"))
asg.set_fragment(sym, FragmentText::from("foo")).unwrap();
asg.set_fragment(dep, FragmentText::from("bar")).unwrap();
asg.set_fragment(ignored, FragmentText::from("baz"))
.unwrap();
let (_, _) = asg.add_dep_lookup(sym, dep);

View File

@ -257,12 +257,7 @@ where
XmloToken::Fragment(sym, text, _),
) => {
istate = IS::None;
let frag = self
.lookup(sym)
.ok_or(AsgBuilderError::MissingFragmentIdent(sym))?;
self.set_fragment(frag, text)?;
self.set_fragment(sym, text)?;
}
// We don't need to read any further than the end of the
@ -301,9 +296,6 @@ pub enum AsgBuilderError {
/// [`Asg`] operation error.
AsgError(AsgError),
/// Fragment encountered for an unknown identifier.
MissingFragmentIdent(SymbolId),
/// Eligibility classification references unknown identifier.
///
/// This is generated by the compiler and so should never happen.
@ -318,12 +310,6 @@ impl Display for AsgBuilderError {
Self::IdentKindError(e) => e.fmt(fmt),
Self::AsgError(e) => e.fmt(fmt),
Self::MissingFragmentIdent(name) => write!(
fmt,
"encountered fragment for unknown identifier `{}`",
name,
),
Self::BadEligRef(name) => write!(
fmt,
"internal error: package elig references nonexistant symbol `{}`",
@ -827,11 +813,8 @@ mod test {
// Note: missing `SymDecl`.
let evs = vec![Ok(XmloToken::Fragment(sym, "foo".into(), DUMMY_SPAN))];
let result = sut
.import_xmlo(evs.into_iter(), SutState::new())
sut.import_xmlo(evs.into_iter(), SutState::new())
.expect_err("expected error for fragment without ident");
assert_eq!(AsgBuilderError::MissingFragmentIdent(sym), result,);
}
#[test]