Mike Gerwitz

Activist for User Freedom

aboutsummaryrefslogtreecommitdiffstats
path: root/tamer
diff options
context:
space:
mode:
authorMike Gerwitz <mike.gerwitz@ryansg.com>2020-04-02 16:18:20 -0400
committerMike Gerwitz <mike.gerwitz@ryansg.com>2020-04-06 09:55:54 -0400
commit0868453dab210405d06a1592a09c22fc273c1d99 (patch)
tree42da0987ef4b55a8989fef80614eec78635bc5d6 /tamer
parenta4657580caf17e0fadfffb7d219f35158a8a7a78 (diff)
downloadtame-0868453dab210405d06a1592a09c22fc273c1d99.tar.gz
tame-0868453dab210405d06a1592a09c22fc273c1d99.tar.bz2
tame-0868453dab210405d06a1592a09c22fc273c1d99.zip
[DEV-7086] Proper handling of identifier overrides
This is an awkward system that I'd like to remove at some point. It adds complexity. For the meantime, overrides have been arbitrarily restricted to a single override (no override-override). But it's needed being until we rework maps and can handle the illusion of overrides using the template system.
Diffstat (limited to 'tamer')
-rw-r--r--tamer/benches/asg.rs59
-rw-r--r--tamer/src/ir/asg/object.rs569
2 files changed, 521 insertions, 107 deletions
diff --git a/tamer/benches/asg.rs b/tamer/benches/asg.rs
index 219d758..f3862c1 100644
--- a/tamer/benches/asg.rs
+++ b/tamer/benches/asg.rs
@@ -461,6 +461,65 @@ mod object {
}
#[bench]
+ fn resolve_1_000_override(bench: &mut Bencher) {
+ let interner = DefaultInterner::new();
+ let sym = interner.intern("sym");
+
+ bench.iter(|| {
+ (0..1000)
+ .map(|_| {
+ Sut::declare(&sym)
+ .resolve(
+ IdentKind::Meta,
+ Source {
+ virtual_: true,
+ ..Default::default()
+ },
+ )
+ .unwrap()
+ .resolve(
+ IdentKind::Meta,
+ Source {
+ override_: true,
+ ..Default::default()
+ },
+ )
+ })
+ .for_each(drop);
+ });
+ }
+
+ // Override encountered before virtual
+ #[bench]
+ fn resolve_1_000_override_virt_after_override(bench: &mut Bencher) {
+ let interner = DefaultInterner::new();
+ let sym = interner.intern("sym");
+
+ bench.iter(|| {
+ (0..1000)
+ .map(|_| {
+ Sut::declare(&sym)
+ .resolve(
+ IdentKind::Meta,
+ Source {
+ override_: true,
+ ..Default::default()
+ },
+ )
+ .unwrap()
+ .resolve(
+ IdentKind::Meta,
+ Source {
+ virtual_: true,
+ ..Default::default()
+ },
+ )
+ })
+ .for_each(drop);
+ });
+ }
+
+ #[bench]
fn set_fragment_1_000_resolved_with_new_str(bench: &mut Bencher) {
let interner = DefaultInterner::new();
let sym = interner.intern("sym");
diff --git a/tamer/src/ir/asg/object.rs b/tamer/src/ir/asg/object.rs
index fd9fb55..8b42fa8 100644
--- a/tamer/src/ir/asg/object.rs
+++ b/tamer/src/ir/asg/object.rs
@@ -251,22 +251,79 @@ impl<'i> IdentObjectState<'i, IdentObject<'i>> for IdentObject<'i> {
/// (it returns to a [`IdentObject::Ident`])
/// to make way for the fragment of the override.
///
+ /// Overrides will always have their virtual flag cleared,
+ /// even if set.
+ /// The compiler will hopefully have done this for us,
+ /// since the user may be confused with subsequent
+ /// [`TransitionError::NonVirtualOverride`] errors if they try to
+ /// override an override.
+ ///
/// The kind of identifier cannot change,
/// but the argument is provided here for convenience so that the
/// caller does not need to perform such a check itself.
fn resolve(
- mut self,
+ self,
kind: IdentKind,
- src: Source<'i>,
+ mut src: Source<'i>,
) -> TransitionResult<IdentObject<'i>> {
match self {
- IdentObject::Ident(_, _, ref mut orig_src)
- if orig_src.virtual_ && src.override_ =>
+ IdentObject::Ident(name, ref orig_kind, ref orig_src)
+ | IdentObject::IdentFragment(
+ name,
+ ref orig_kind,
+ ref orig_src,
+ _,
+ ) if src.override_ => {
+ if !orig_src.virtual_ {
+ let err = TransitionError::NonVirtualOverride {
+ name: name.to_string(),
+ };
+
+ return Err((self, err));
+ }
+
+ if orig_kind != &kind {
+ let err = TransitionError::VirtualOverrideKind {
+ name: name.to_string(),
+ existing: orig_kind.clone(),
+ given: kind.clone(),
+ };
+
+ return Err((self, err));
+ }
+
+ // Ensure that virtual flags are cleared to prohibit
+ // override-overrides. The compiler should do this; this is
+ // just an extra layer of defense.
+ src.virtual_ = false;
+
+ // Note that this has the effect of clearing fragments if we
+ // originally were in state `IdentObject::IdentFragment`.
+ Ok(IdentObject::Ident(name, kind, src))
+ }
+
+ // If we encountered the override _first_, flip the context by
+ // declaring a new identifier and trying to override that.
+ IdentObject::Ident(name, orig_kind, orig_src)
+ if orig_src.override_ =>
{
- *orig_src = src;
- Ok(self)
+ Self::declare(name)
+ .resolve(kind, src)?
+ .resolve(orig_kind, orig_src)
}
+ // Same as above, but for fragments, we want to keep the
+ // _original override_ fragment.
+ IdentObject::IdentFragment(
+ name,
+ orig_kind,
+ orig_src,
+ orig_text,
+ ) if orig_src.override_ => Self::declare(name)
+ .resolve(kind, src)?
+ .resolve(orig_kind, orig_src)?
+ .set_fragment(orig_text),
+
IdentObject::Extern(name, ref orig_kind, _) => {
if orig_kind != &kind {
let err = TransitionError::ExternResolution {
@@ -281,14 +338,6 @@ impl<'i> IdentObjectState<'i, IdentObject<'i>> for IdentObject<'i> {
Ok(IdentObject::Ident(name, kind, src))
}
- // TODO: no override-override
- IdentObject::IdentFragment(name, _, orig_src, _)
- if orig_src.virtual_ && src.override_ =>
- {
- // clears fragment, which is no longer applicable
- Ok(IdentObject::Ident(name, kind, src))
- }
-
IdentObject::Missing(name) | IdentObject::Ident(name, _, _) => {
Ok(IdentObject::Ident(name, kind, src))
}
@@ -331,6 +380,19 @@ impl<'i> IdentObjectState<'i, IdentObject<'i>> for IdentObject<'i> {
Ok(IdentObject::IdentFragment(sym, kind, src, text))
}
+ // If we get to this point in a properly functioning program (at
+ // least as of the time of writing), then we have encountered a
+ // fragment for a virtual identifier _after_ we have already
+ // encountered the fragment for its _override_. We therefore
+ // want to keep the override.
+ //
+ // If this is not permissable, then we should have already
+ // prevented the `resolve` transition before this fragment was
+ // encountered.
+ IdentObject::IdentFragment(_, _, ref src, _) if src.override_ => {
+ Ok(self)
+ }
+
IdentObject::IdentFragment(_, IdentKind::MapHead, _, _)
| IdentObject::IdentFragment(_, IdentKind::MapTail, _, _)
| IdentObject::IdentFragment(_, IdentKind::RetMapHead, _, _)
@@ -338,40 +400,6 @@ impl<'i> IdentObjectState<'i, IdentObject<'i>> for IdentObject<'i> {
Ok(self)
}
- // TODO remove these ignores when fixed
- IdentObject::IdentFragment(
- sym,
- IdentKind::Map,
- Source {
- virtual_: true,
- override_: true,
- ..
- },
- _,
- ) => {
- eprintln!(
- "ignoring virtual and overridden map object: {}",
- sym
- );
- Ok(self)
- }
- IdentObject::IdentFragment(
- sym,
- IdentKind::RetMap,
- Source {
- virtual_: true,
- override_: true,
- ..
- },
- _,
- ) => {
- eprintln!(
- "ignoring virtual and overridden retmap object: {}",
- sym
- );
- Ok(self)
- }
-
_ => {
let msg = format!(
"identifier is not a IdentObject::Ident): {:?}",
@@ -400,6 +428,17 @@ pub enum TransitionError {
given: IdentKind,
},
+ /// Attempt to override a non-virtual identifier.
+ NonVirtualOverride { name: String },
+
+ /// Overriding a virtual identifier failed due to an incompatible
+ /// [`IdentKind`].
+ VirtualOverrideKind {
+ name: String,
+ existing: IdentKind,
+ given: IdentKind,
+ },
+
/// The provided identifier is not in a state that is permitted to
/// receive a fragment.
///
@@ -420,6 +459,22 @@ impl std::fmt::Display for TransitionError {
name, expected, given,
),
+ Self::NonVirtualOverride { name } => write!(
+ fmt,
+ "non-virtual identifier `{}` cannot be overridden",
+ name,
+ ),
+
+ Self::VirtualOverrideKind {
+ name,
+ existing,
+ given,
+ } => write!(
+ fmt,
+ "virtual identifier `{}` of type `{}` cannot be overridden with type `{}`",
+ name, existing, given,
+ ),
+
Self::BadFragmentDest(msg) => {
write!(fmt, "bad fragment destination: {}", msg)
}
@@ -947,76 +1002,376 @@ mod test {
}
}
- // TODO: incompatible
- #[test]
- fn declare_override_virtual_ident() {
- let sym = symbol_dummy!(1, "virtual");
- let over_src = symbol_dummy!(2, "src");
- let kind = IdentKind::Meta;
+ mod override_ {
+ use super::*;
- let virt = IdentObject::declare(&sym)
- .resolve(
- kind.clone(),
- Source {
- virtual_: true,
- ..Default::default()
- },
- )
- .unwrap();
+ #[test]
+ fn declare_virtual_ident_first() {
+ let sym = symbol_dummy!(1, "virtual");
+ let over_src = symbol_dummy!(2, "src");
+ let kind = IdentKind::Meta;
+
+ let virt = IdentObject::declare(&sym)
+ .resolve(
+ kind.clone(),
+ Source {
+ virtual_: true,
+ ..Default::default()
+ },
+ )
+ .unwrap();
- let over_src = Source {
- override_: true,
- src: Some(&over_src),
- ..Default::default()
- };
+ let over_src = Source {
+ virtual_: true, // this needn't be set, but see below
+ override_: true,
+ src: Some(&over_src),
+ ..Default::default()
+ };
- let result = virt.resolve(kind.clone(), over_src.clone());
+ let result = virt.resolve(kind.clone(), over_src.clone());
- assert_eq!(Ok(IdentObject::Ident(&sym, kind, over_src)), result);
- }
+ // Overriding should clear any virtual flag that may have
+ // been set to prevent override-overrides.
+ let expected_src = Source {
+ virtual_: false,
+ ..over_src
+ };
- // TODO: incompatible
- #[test]
- fn declare_override_virtual_ident_fragment() {
- let sym = symbol_dummy!(1, "virtual");
- let over_src = symbol_dummy!(2, "src");
- let kind = IdentKind::Meta;
+ assert_eq!(
+ Ok(IdentObject::Ident(&sym, kind, expected_src)),
+ result
+ );
+ }
- let virt_src = Source {
- virtual_: true,
- ..Default::default()
- };
+ // Override is encountered before the virtual
+ #[test]
+ fn declare_virtual_ident_after_override() {
+ let sym = symbol_dummy!(1, "virtual_second");
+ let virt_src = symbol_dummy!(2, "virt_src");
+ let kind = IdentKind::Meta;
- let virt = IdentObject::declare(&sym)
- .resolve(kind.clone(), virt_src.clone())
- .unwrap();
- let text = FragmentText::from("remove me");
- let virt_frag = virt.set_fragment(text.clone());
+ let over_src = Source {
+ virtual_: true, // this needn't be set, but see below
+ override_: true,
+ ..Default::default()
+ };
- assert_eq!(
- Ok(IdentObject::IdentFragment(
- &sym,
- kind.clone(),
- virt_src,
- text
- )),
- virt_frag,
- );
+ let over = IdentObject::declare(&sym)
+ .resolve(kind.clone(), over_src.clone())
+ .unwrap();
- let over_src = Source {
- override_: true,
- src: Some(&over_src),
- ..Default::default()
- };
+ let virt_src = Source {
+ virtual_: true,
+ src: Some(&virt_src),
+ ..Default::default()
+ };
+
+ let result = over.resolve(kind.clone(), virt_src.clone());
+
+ // Overriding should clear any virtual flag that may have
+ // been set to prevent override-overrides. We should also
+ // take the override source even though virtual was second.
+ let expected_src = Source {
+ virtual_: false,
+ ..over_src
+ };
+
+ assert_eq!(
+ Ok(IdentObject::Ident(&sym, kind, expected_src)),
+ result
+ );
+ }
- let result =
- virt_frag.unwrap().resolve(kind.clone(), over_src.clone());
+ #[test]
+ fn declare_override_non_virtual() {
+ let sym = symbol_dummy!(1, "non_virtual");
+ let kind = IdentKind::Meta;
+
+ let non_virt = IdentObject::declare(&sym)
+ .resolve(
+ kind.clone(),
+ Source {
+ virtual_: false,
+ ..Default::default()
+ },
+ )
+ .unwrap();
- // The act of overriding the object should have cleared any
- // existing fragment, making way for a new fragment to take its
- // place as soon as it is discovered. (So, back to an
- // IdentObject::Ident.)
- assert_eq!(Ok(IdentObject::Ident(&sym, kind, over_src)), result);
+ let over_src = Source {
+ override_: true,
+ ..Default::default()
+ };
+
+ // This isn't the purpose of the test, but we want to make
+ // sure that the non-virtual override error occurs before
+ // the kind error.
+ let bad_kind = IdentKind::Cgen(Dim::from_u8(1));
+
+ let result = non_virt
+ .clone()
+ .resolve(bad_kind, over_src.clone())
+ .expect_err("expected error");
+
+ match result {
+ (
+ ref orig,
+ TransitionError::NonVirtualOverride { ref name },
+ ) => {
+ assert_eq!(orig, &non_virt);
+ assert_eq!(*sym, *name);
+
+ // Formatted error
+ let (_, err) = result;
+ let msg = format!("{}", err);
+
+ assert!(msg.contains(&format!("{}", sym)));
+ }
+ (_, TransitionError::VirtualOverrideKind { .. }) => {
+ panic!("kind check must happen _after_ virtual check")
+ }
+ _ => panic!(
+ "expected TransitionError::VirtualOverrideKind {:?}",
+ result
+ ),
+ }
+ }
+
+ #[test]
+ fn declare_virtual_ident_incompatible_kind() {
+ let sym = symbol_dummy!(1, "virtual");
+ let src_sym = symbol_dummy!(2, "src");
+ let kind = IdentKind::Meta;
+
+ let virt = IdentObject::declare(&sym)
+ .resolve(
+ kind.clone(),
+ Source {
+ virtual_: true,
+ ..Default::default()
+ },
+ )
+ .unwrap();
+
+ let over_src = Source {
+ override_: true,
+ src: Some(&src_sym),
+ ..Default::default()
+ };
+
+ let bad_kind = IdentKind::Cgen(Dim::from_u8(1));
+ let result = virt
+ .clone()
+ .resolve(bad_kind.clone(), over_src.clone())
+ .expect_err("expected error");
+
+ match result {
+ (
+ ref orig,
+ TransitionError::VirtualOverrideKind {
+ ref name,
+ ref existing,
+ ref given,
+ },
+ ) => {
+ assert_eq!(orig, &virt);
+
+ assert_eq!(*sym, *name);
+ assert_eq!(&kind, existing);
+ assert_eq!(&bad_kind, given);
+
+ // Formatted error
+ let (_, err) = result;
+ let msg = format!("{}", err);
+
+ assert!(msg.contains(&format!("{}", sym)));
+ assert!(msg.contains(&format!("{}", kind)));
+ assert!(msg.contains(&format!("{}", bad_kind)));
+ }
+ _ => panic!(
+ "expected TransitionError::VirtualOverrideKind {:?}",
+ result
+ ),
+ }
+ }
+
+ // Encounter virtual first and override second should cause the
+ // fragment to be cleared to make way for the new fragment.
+ #[test]
+ fn declare_override_virtual_ident_fragment_virtual_first() {
+ let sym = symbol_dummy!(1, "virtual");
+ let over_src = symbol_dummy!(2, "src");
+ let kind = IdentKind::Meta;
+
+ // Remember: override is going to come first...
+ let over_src = Source {
+ override_: true,
+ src: Some(&over_src),
+ ..Default::default()
+ };
+
+ // ...and virt second.
+ let virt_src = Source {
+ virtual_: true,
+ ..Default::default()
+ };
+
+ let over = IdentObject::declare(&sym)
+ .resolve(kind.clone(), over_src.clone())
+ .unwrap();
+
+ // So we should _keep_ this fragment, since it represent the
+ // override, even though it's appearing first.
+ let text = FragmentText::from("keep me");
+ let over_frag = over.set_fragment(text.clone());
+
+ assert_eq!(
+ Ok(IdentObject::IdentFragment(
+ &sym,
+ kind.clone(),
+ over_src.clone(),
+ text.clone(),
+ )),
+ over_frag,
+ );
+
+ let result =
+ over_frag.unwrap().resolve(kind.clone(), virt_src.clone());
+
+ // Overriding should _not_ have cleared the fragment since
+ // the override was encountered _first_, so we want to keep
+ // its fragment.
+ assert_eq!(
+ Ok(IdentObject::IdentFragment(
+ &sym,
+ kind.clone(),
+ over_src.clone(),
+ text.clone()
+ )),
+ result
+ );
+
+ // Finally, after performing this transition, we will
+ // inevitably encounter the fragment for the virtual
+ // identifier, which we must ignore. So we must make sure
+ // that encountering it will not cause an error, because we
+ // still have an IdentFragment at this point.
+ assert_eq!(
+ Ok(IdentObject::IdentFragment(
+ &sym,
+ kind,
+ over_src.clone(),
+ text.clone()
+ )),
+ result.unwrap().set_fragment("virt fragment".into()),
+ );
+ }
+
+ // Encountering _override_ first and virtual second should _not_
+ // clear the fragment, otherwise the virtual fragment will take
+ // precedence over the override.
+ #[test]
+ fn declare_override_virtual_ident_fragment_override_first() {
+ let sym = symbol_dummy!(1, "virtual");
+ let over_src = symbol_dummy!(2, "src");
+ let kind = IdentKind::Meta;
+
+ let virt_src = Source {
+ virtual_: true,
+ ..Default::default()
+ };
+
+ let virt = IdentObject::declare(&sym)
+ .resolve(kind.clone(), virt_src.clone())
+ .unwrap();
+ let text = FragmentText::from("remove me");
+ let virt_frag = virt.set_fragment(text.clone());
+
+ assert_eq!(
+ Ok(IdentObject::IdentFragment(
+ &sym,
+ kind.clone(),
+ virt_src,
+ text
+ )),
+ virt_frag,
+ );
+
+ let over_src = Source {
+ override_: true,
+ src: Some(&over_src),
+ ..Default::default()
+ };
+
+ let result =
+ virt_frag.unwrap().resolve(kind.clone(), over_src.clone());
+
+ // The act of overriding the object should have cleared any
+ // existing fragment, making way for a new fragment to take its
+ // place as soon as it is discovered. (So, back to an
+ // IdentObject::Ident.)
+ assert_eq!(
+ Ok(IdentObject::Ident(&sym, kind, over_src)),
+ result
+ );
+ }
+
+ #[test]
+ fn declare_override_virtual_ident_fragment_incompatible_type() {
+ let sym = symbol_dummy!(1, "virtual");
+ let over_src = symbol_dummy!(2, "src");
+ let kind = IdentKind::Meta;
+
+ let virt_src = Source {
+ virtual_: true,
+ ..Default::default()
+ };
+
+ let virt = IdentObject::declare(&sym)
+ .resolve(kind.clone(), virt_src.clone())
+ .unwrap();
+ let virt_frag = virt.set_fragment("".into()).unwrap();
+
+ let over_src = Source {
+ override_: true,
+ src: Some(&over_src),
+ ..Default::default()
+ };
+
+ let bad_kind = IdentKind::Cgen(Dim::from_u8(1));
+ let result = virt_frag
+ .clone()
+ .resolve(bad_kind.clone(), over_src.clone())
+ .expect_err("expected error");
+
+ match result {
+ (
+ ref orig,
+ TransitionError::VirtualOverrideKind {
+ ref name,
+ ref existing,
+ ref given,
+ },
+ ) => {
+ assert_eq!(orig, &virt_frag);
+
+ assert_eq!(*sym, *name);
+ assert_eq!(&kind, existing);
+ assert_eq!(&bad_kind, given);
+
+ // Formatted error
+ let (_, err) = result;
+ let msg = format!("{}", err);
+
+ assert!(msg.contains(&format!("{}", sym)));
+ assert!(msg.contains(&format!("{}", kind)));
+ assert!(msg.contains(&format!("{}", bad_kind)));
+ }
+ _ => panic!(
+ "expected TransitionError::VirtualOverrideKind {:?}",
+ result
+ ),
+ }
+ }
}
fn add_ident_kind_ignores(given: IdentKind, expected: IdentKind) {