From 0b9e91b936a9f1380735f27abc86286b1af9738d Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike.gerwitz@ryansg.com>
Date: Tue, 6 Jun 2023 15:12:56 -0400
Subject: [PATCH] tamer: obj::xmlo::reader::XmloReader: Remove generics

This cleanup is an interesting one, because I think the present me may
disagree with the past me.

The use of generics here to compose the parser from smaller parsers was due
to how I wrote my object-oriented code in other languages: where a class was
an independently tested unit.  I was trying to reproduce the same here,
utilizing generics in the same way that one would use compoisition via
object constructors in other languages.

But it's been a long time since then, and I've come to settle on different
standards in Rust.  The components of `XmloReader` really are just
implementation details.  As I find myself about to want to modify its
behavior, I don't _want_ to compose `XmloReader` from _different_ parsers;
that may result in an invalid parse.  There's one correct way to parse an
xmlo file.

If I want to parse the file differently, then `XmloReader` ought to expose
a way of doing so.  This is more rigid, but that rigidity buys us confidence
that the system has been explicitly designed to support those
operations.  And that confidence gives us peace of mind knowing that the
system won't compose in ways that we don't intend for it to.

Of course, I _could_ design the system to compose in generic ways.  But
that's an over-generalization that I don't think will be helpful; it's not
only a greater cognitive burden, but it's also a lot more work to ensure
that invariants are properly upheld and to design an API that will ensure
that parsing is always correct.  It's simply not worth it.

So, this makes `XmloReader` consistent with other parsers now, like
`AirAggregate` and nir::parse (ele_parse).  This prepares for a change to
make `XmloReader` configurable to avoid loading fragments from object files,
since that's very wasteful for `tamec`.

DEV-13162
---
 tamer/src/obj/xmlo/reader.rs | 44 +++++++++++++-----------------------
 1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/tamer/src/obj/xmlo/reader.rs b/tamer/src/obj/xmlo/reader.rs
index 50b1f61e..59e8a652 100644
--- a/tamer/src/obj/xmlo/reader.rs
+++ b/tamer/src/obj/xmlo/reader.rs
@@ -25,8 +25,8 @@ use crate::{
     num::{Dim, Dtype},
     obj::xmlo::SymType,
     parse::{
-        self, util::SPair, ClosedParseState, EmptyContext, NoContext,
-        ParseState, Token, Transition, TransitionResult, Transitionable,
+        self, util::SPair, NoContext, ParseState, Token, Transition,
+        TransitionResult, Transitionable,
     },
     span::Span,
     sym::{st::raw, GlobalSymbolIntern, GlobalSymbolResolve, SymbolId},
@@ -163,44 +163,30 @@ impl Display for XmloToken {
     }
 }
 
-/// A parser capable of being composed with [`XmloReader`].
-pub trait XmloState =
-    ClosedParseState<Token = Xirf<Text>, Context = EmptyContext>
-    where
-        Self: Default,
-        <Self as ParseState>::Error: Into<XmloError>,
-        <Self as ParseState>::Object: Into<XmloToken>;
-
 #[derive(Debug, Default, PartialEq, Eq)]
-pub enum XmloReader<
-    SS: XmloState = SymtableState,
-    SD: XmloState = SymDepsState,
-    SF: XmloState = FragmentsState,
-> {
+pub enum XmloReader {
     /// Parser has not yet processed any input.
     #[default]
     Ready,
     /// Processing `package` attributes.
     Package(Span),
     /// Expecting a symbol declaration or closing `preproc:symtable`.
-    Symtable(Span, SS),
+    Symtable(Span, SymtableState),
     /// Symbol dependencies are expected next.
     SymDepsExpected,
     /// Expecting symbol dependency list or closing `preproc:sym-deps`.
-    SymDeps(Span, SD),
+    SymDeps(Span, SymDepsState),
     /// Compiled text fragments are expected next.
     FragmentsExpected,
     /// Expecting text fragment or closing `preproc:fragments`.
-    Fragments(Span, SF),
+    Fragments(Span, FragmentsState),
     /// End of header parsing.
     Eoh,
     /// `xmlo` file has been fully read.
     Done,
 }
 
-impl<SS: XmloState, SD: XmloState, SF: XmloState> ParseState
-    for XmloReader<SS, SD, SF>
-{
+impl ParseState for XmloReader {
     type Token = Xirf<Text>;
     type Object = XmloToken;
     type Error = XmloError;
@@ -249,7 +235,7 @@ impl<SS: XmloState, SD: XmloState, SF: XmloState> ParseState
             (Package(_), Xirf::Close(..)) => Transition(Done).incomplete(),
 
             (Package(_), Xirf::Open(QN_P_SYMTABLE, span, ..)) => {
-                Transition(Symtable(span.tag_span(), SS::default()))
+                Transition(Symtable(span.tag_span(), SymtableState::default()))
                     .incomplete()
             }
 
@@ -269,7 +255,8 @@ impl<SS: XmloState, SD: XmloState, SF: XmloState> ParseState
             ),
 
             (SymDepsExpected, Xirf::Open(QN_P_SYM_DEPS, span, _)) => {
-                Transition(SymDeps(span.tag_span(), SD::default())).incomplete()
+                Transition(SymDeps(span.tag_span(), SymDepsState::default()))
+                    .incomplete()
             }
 
             (SymDeps(_, sd), Xirf::Close(None | Some(QN_P_SYM_DEPS), ..))
@@ -286,8 +273,11 @@ impl<SS: XmloState, SD: XmloState, SF: XmloState> ParseState
             ),
 
             (FragmentsExpected, Xirf::Open(QN_P_FRAGMENTS, span, _)) => {
-                Transition(Fragments(span.tag_span(), SF::default()))
-                    .incomplete()
+                Transition(Fragments(
+                    span.tag_span(),
+                    FragmentsState::default(),
+                ))
+                .incomplete()
             }
 
             (
@@ -343,9 +333,7 @@ fn canonical_slash(name: SymbolId) -> SymbolId {
     }
 }
 
-impl<SS: XmloState, SD: XmloState, SF: XmloState> Display
-    for XmloReader<SS, SD, SF>
-{
+impl Display for XmloReader {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         use XmloReader::*;