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

1 files changed, 462 insertions, 107 deletions

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) {