Just some final POC setup for how this'll work; it's nothing
significant. This just emits an `@xmlns` on the `package` element to
demonstrate use of the stack.
With that, it's time to formalize this.
I also need to document at some point why I choose to use `ArrayVec` still
over `Vec`---it's not a microoptimization. It's intended to simplify the
runtime to keep execution simple with fewer code paths and make it more
amenable to analysis. Memory allocation is a pretty complex thing and
muddies execution. It's also another point of failure, though practically
speaking, I'm not worried about that---this is replacing a system that
consumes many GiB of memory (XSLT-based compiler) with one that consumes 10s
of MiB.
DEV-13708
This (a) hold the state of a stack that I can populate with tokens rather
than introducing a state for every single e.g. attribute and such on
elements (so, more like the `xmle` XIR lowering).
It also hides the obvious awkwardness of the `&mut &'a Asg`, but that's not
the intent of it.
DEV-13708
This is just a special case of lowering with a context, and maintaining two
separate implementations has resulted in divergence. I don't recall why I
didn't do this previously, though it's possible that the lowering pipeline
was in a state that made it more difficult to do (e.g. with error
handling).
DEV-13708
Technically, an "acceptor" in the context of state machines is actually a
state machine; the terminology here is more describing the configuration of
the state machine (`XirToXirf`) as an acceptor.
This change comes with significant documentation of the rationale and why
this is important; see that for more information.
This change is necessary so that we can enforce finalization on all parsers
in the lowering pipeline, which is not currently being done. If we were to
do that now, then `tameld` would fail because it halts parsing of the tokens
stream at the end of the `xmlo` header.
This is also quite the type soup, but I'm not going to refine this further
right now, since my focus is elsewhere (XMLI lowering).
DEV-13708
This has been a long time coming. The wiring of it all together is a little
rough around the edges right now, but this commit represents a working POC
to begin to fill in the gaps for the entire lowering pipeline.
I had hoped to be at this point a year ago. Yeah.
This marks a significant milestone in the project because this allows me to
begin to observe the implementation end-to-end, testing it on real-life
inputs as part of a production build pipeline.
...and now, with that, we can begin. So much work has gone into this
project so far, but aside from the linker (which has been in production for
years), most of this work has been foundational. It's been a significant
investment that I intend to have pay off in many different ways.
(All this outputs right now is `<package/>`.)
DEV-13708
This replaces the stub `derive_xmli` with the same result (well, minus a
space before the '/' in the output) using what will become the lowering
pipeline. Once again, this is quite verbose, and the lowering pipeline in
general needs to be further abstracted away.
Unlike the rest of the pipeline, an error during the derivation process will
immediately terminate with an unrecoverable error, because we do not want to
write partial files. This does not remove the garbage file, because the
build system ought to do that itself (e.g. `make`)...but that is certainly
open for debate.
DEV-13708
The reader previously yielded a `ParsedResult`, presumably to simplify
lowering operations. But the reader is not a `ParseState`, and does not
otherwise use the parsing API, so this was an inappropriate and confusing
coupling.
This resolves that, introducing a new `lowerable` which will translate an
iterator into something that can be placed in a lowering pipeline.
See the previous commit for more information.
DEV-13708
The token type was previously hard-coded to `UnknownToken`, since the use
case was the beginning of the lowering pipeline at the start of the program,
where there was no token type because the first parser (`XirReader`,
currently) is responsible for producing the first token type.
But when we're lowering from the graph (so, the other side of the lowering
pipeline), we _do_ have token types to deal with.
This also emphasizes the inappropriate coupling of `<XirReader as
Iterator>::Item` with `ParsedResult`; I'd like to follow the same approach
that I'm about to introduce with `tamec`, so see a future commit.
DEV-13708
This was missed (because it was not used) when EOF tokens were originally
introduced via `ParseState::eof_tok`---`LowerIter` also needs to consider
the token.
This separation betwen the two iterators is a maintenance burden that needs
to be taken care of; I knew that at the time, and then I forgot about it,
and here we are.
This was caught while beginning to wire together a POC graph lowering
pipeline to emit derived sources.
DEV-13708
This parser does exactly what it says it does. Its implementation is
simple, but I added a test anyway just to prove that it works, and the test
seems more complicated than the implementation itself, given the types
involved.
DEV-13708
This introduces a `Token` in place of the original tuple for
`TreePreOrderDfs` so that it can be used as input to a parser that will
lower into XIRF.
This requires that various things be describable (using `Display`), which
this also adds. This is an example of where the parsing framework itself
enforces system observability by ensuring that every part of the system can
describe its state.
DEV-13708
This lowering operation is intended to allow me to write a more concise and
clear mapping from the graph to XIRF, without having to worry about
balancing tags, which really complicated the implementation.
This has details docs; see that for more information.
I can't help but be reminded of Wisp (the whitespace-based Lisp-like
syntax). Which is unfortunate, because I'm not fond of Wisp; I like my
parenthesis.
DEV-13708
The `TreePreOrderDfs` iterator needed to expose additional edge context to
the caller (specifically, the `Span`). This was getting a bit messy, so
this consolodates everything into a new `DynObjectRel`, which also
emphasizes that it is in need of narrowing.
Packing everything up like that also allows us to return more information to
the caller without complicating the API, since the caller does not need to
be concerned with all of those values individually.
Depth is kept separate, since that is a property of the traversal and is not
stored on the graph. (Rather, it _is_ a property of the graph, but it's not
calculated until traversal. But, depth will also vary for a given node
because of cross edges, and so we cannot store any concrete depth on the
graph for a given node. Not even a canonical one, because once we start
doing inlining and common subexpression elimination, there will be shared
edges that are _not_ cross edges (the node is conceptually part of _both_
trees). Okay, enough of this rambling parenthetical.)
DEV-13708
This information is necessary to be able to reconstruct the tree, since
the `ObjectIndex` alone does not give you enough information. Even if you
inspected the graph, it _still_ wouldn't give you enough information, since
you don't know the current path of the traversal for nodes that may have
multiple incoming edges. (Any assumptions you could make today won't
always be valid in the future.)
DEV-13708
This begins to introduce a graph traversal useful for a source
reconstruction from the current state of the ASG. The idea is to, after
having parsed and ingested the source through the lowering pipeline, to
re-output it to (a) prove that we have parsed correctly and (b) allow
progressively moving things from the XSLT-based compiler into TAMER.
There's quite a bit of documentation here; see that for more
information. Generalizing this in an appropriate way took some time, but I
think this makes sense (that work began with the introduction of cross edges
in terms of the tree described by the graph's ontology). But I do need to
come up with an illustration to include in the documentation.
DEV-13708
The `Pkg` span will now properly reflect the entire definition of the
package including the opening and closing tags.
This was found while I was working on a graph traversal.
DEV-13597
I noticed this while working on a graph traversal. The unit test used the
same span for both the reference _and_ the binding, so I didn't notice. -_-
The problem with this, though, is that we do not have a separate span
representing the source location of the identifier reference. The reason is
that we decided to re-use an existing node rather than creating another one,
which would add another inconvenient layer of indirection (and complexity).
So, I may have to add (optional?) spans to edges.
DEV-13708
This introduces the concept of ontological cross edges.
The term "cross edge" is most often seen in the context of graph traversals,
e.g. the trees formed by a depth-first search. This, however, refers to the
trees that are inherent in the ontology of the graph.
For example, an `ExprRef` will produce a cross edge to the referenced
`Ident`, that that is a different tree than the current expression. (Well,
I suppose technically it _could_ be a back edge, but then that'd be a cycle
which would fail the process once we get to preventing it. So let's ignore
that for now.)
DEV-13708
This causes a package definition to be rooted (so that it can be easily
accessed for a graph walk). This keeps consistent with the new
`ObjectIndex`-based API by introducing a unit `Root` `ObjectKind` and the
boilerplate that goes with it.
This boilerplate, now glaringly obvious, will be refactored at some point,
since its repetition is onerous and distracting.
DEV-13159
Included in this diff are the corresponding changes to the graph to support
the change. Adding the edge was easy, but we also need a way to get the
package for an identifier. The easiest way to do that is to modify the edge
weight to include not just the target node type, but also the source.
DEV-13159
This does not yet create edges from identifiers to the package; just getting
this introduced was quite a bit of work, so I want to get this committed.
Note that this also includes a change to NIR so that `Close` contains the
entity so that we can pattern-match for AIR transformations rather than
retaining yet another stack with checks that are already going to be done by
AIR. This makes NIR stand less on its own from a self-validation point, but
that's okay, given that it's the language that the user entered and,
conceptually, they could enter invalid NIR the same as they enter invalid
XML (e.g. from a REPL).
In _practice_, of course, NIR is lowered from XML and the schema is enforced
during that lowering and so the validation does exist as part of that
parsing.
These concessions speak more to the verbosity of the language (Rust) than
anything.
DEV-13159
Rather than panicing at this level, let's panic at the caller, simplifying
impls and keeping them total.
This can't occur now, but an upcoming change introducing a package type will
allow for such a thing.
DEV-13159
This hides information that's taking up a lot of space in the parser traces
and is not useful information. In particular, the `index` contains a lot of
empty space due to pre-interned symbols.
The index was going to be converted into a HashMap, but that was reverted
because the tradeoff did not make sense, and so this problem remains; see
the previous commit for more information.
DEV-13159
This reverts commit 1b7eac337cd5909c01ede3a5b3fba577898d5961.
I don't actually think this ends up being worth it in the end. Sure, the
implementation is simpler at a glance, but it is more complex at runtime,
adding more cycles for little benefit.
There are ~220 pre-interned symbols at the time of writing, so ~880 bytes (4
bytes per symbol) are potentially wasted if _none_ of the pre-interned
symbols end up serving as identifiers in the graph. The reality is that
some of them _will_ but, but using HashMap also introduces overhead, so in
practice, the savings is much less. On a fairly small package, it was <100
bytes memory saving in `tamec`. For `tameld`, it actually uses _more_
memory, especially on larger packages, because there are 10s of thousands of
symbols involved. And we're incurring a rehashing cost on resize, unlike
this original plain `Vec` implementation.
So, I'm leaving this in the history to reference in the future or return to
it if others ask; maybe it'll be worth it in the future.
This was originally written before there were a bunch of preinterned
symbols. Now the index vector is very sparse.
This simplifies things a bit. If this ends up manifesting as a bottleneck
in the future, we can revisit the implementation. While this does result in
more cycles, it's neglibable relative to the total cycle count.
This commit is what I've been sitting on for testing some of the recent
changes; it is a very basic demonstration of lowering all the way down
from source XML files into the ASG. This can be run on real files to
observe, beyond unit tests, how the system reacts.
Once this outputs data from the graph, we'll finally have tamec end-to-end
and can just keep filling the gaps.
I'm hoping to roll the desugaring process into NirToAir rather than having a
separate process as originally planned a couple of months back.
This also introduces the `wip-nir-to-air` feature flag. Currently,
interpolation will cause a `Nir::BindIdent` to be emitted in blocks that
aren't yet emitting NIR, and so results in an invalid parse.
DEV-13159
This adds support for identifier references, adding `Ident` as a valid edge
type for `Expr`.
There is nothing in the system yet to enforce ontology through levels of
indirection; that will come later on.
I'm testing these changes with a very minimal NIR parse, which I'll commit
shortly.
DEV-13597
This was originally created to populate Neo4J for querying, but it has not
been utilized. It's become a maintenance burden as I try to change the API
of and encapsulate the graph, which is important for upholding its
invariants.
This feature, or one like it, will return in the future. I have other
related plans; we'll see if they materialize.
The graph can't be encapsulated fully just yet because of the linker; those
commits will come in the following days.
DEV-13597
This allows for edges to be multiple types, and gives us two important
benefits:
(a) Compiler-verified correctness to ensure that we don't generate graphs
that do not adhere to the ontology; and
(b) Runtime verification of types, so that bugs are still memory safe.
There is a lot more information in the documentation within the patch.
This took a lot of iterating to get something that was tolerable. There's
quite a bit of boilerplate here, and maybe that'll be abstracted away better
in the future as the graph grows.
In particular, it was challenging to determine how I wanted to actually go
about narrowing and looking up edges. Initially I had hoped to represent
the subsets as `ObjectKind`s as well so that you could use them anywhere
`ObjectKind` was expected, but that proved to be far too difficult because I
cannot return a reference to a subset of `Object` (the value would be owned
on generation). And while in a language like C maybe I'd pad structures and
cast between them safely, since they _do_ overlap, I can't confidently do
that here since Rust's discriminant and layout are not under my control.
I tried playing around with `std::mem::Discriminant` as well, but
`discriminant` (the function) requires a _value_, meaning I couldn't get the
discriminant of a static `Object` variant without some dummy value; wasn't
worth it over `ObjectRelTy.` We further can't assign values to enum
variants unless they hold no data. Rust a decade from now may be different
and will be interesting to look back on this struggle.
DEV-13597
We only need a reference to the inner object, for which `AsRef` is the
proper and idiomatic solution.
There is a lot of boilerplate here that I hope to reduce in the future.
DEV-13597
ObjectRelTo is sufficient and, while I originally thought it was useful to
have it read left-to-right, it just ends up being a cognitive burden.
DEV-13597
I'm spending a lot of time considering how the future system will work,
which is complicating the needs of the system now, which is to re-output the
source XML so that we can selectively start to replace things.
So I'm going to punt on this.
I was also planning out how that edge reassignment out to work, along with
traits to try to enforce it, and that is also complicated, so I may wind up
wanting to leave them in the end, or handling this
differently. Specifically, I'll want to know how `value-of` expressions are
going to work on the graph first, since its target is going to be dynamic
and therefore not knowable at compile-time. (Rather, I know how I want to
make them work, but I want to observe that working in practice first.)
DEV-13597
There is extensive rationale in the documentation for this new macro. I'm
utilizing it to provide a more clear and friendly message for incomplete
ident resolution so that I can move on and return to those situations later.
It's worth noting that:
- Externs _will_ need to be handled in the near-term;
- Opaque and IdentFragment almost certainly won't be bound to a definition
until I introduce LTO, which is quite a ways off; and
- They may use the same mechanism and so may be able to be handled at the
same time anyway.
DEV-13597
The ASG delegates certain operations to Objects so that they may enforce
their own invariants and ontology. It is therefore important that only
objects have access to certain methods on `Asg`, otherwise those invariants
could be circumvented.
It should be noted that the nesting of this module is such that AIR should
_not_ have privileged access to the ASG---it too must utilize objects to
ensure those invariants are enforced in a single place.
DEV-13597
Starting to re-organize things to match my mental model of the new system;
the ASG abstraction has changed quite a bit since the early days.
This isn't quite enough, though; see next commit.
DEV-13597
This provides the initial implementation allowing an identifier to be
defined (bound to an object and made transparent).
I'm not yet entirely sure whether I'll stick with the "transparent" and
"opaque" terminology when there's also "declare" and "define", but a
`Missing` state is a type of declaration and so the distinction does still
seem to be important.
There is still work to be done on `ObjectIndex::<Ident>::bind_definition`,
which will follow. I'm going to be balancing work to provide type-level
guarantees, since I don't have the time to go as far as I'd like.
DEV-13597
This seems to have been an oversight from when I recently introduced SPairs
to ASG; I noticed it while working on another change and receiving back a
`DUMMY_SPAN`.
DEV-13597
`Ident` is now `Opaque`, but the new `Transparent` state isn't actually used
yet in any transitions; that'll come next.
The original (now "opaque") identifiers were added for the linker, which
does not need (at present) the associated expressions, since they've already
been compiled. In the future I'd like to do LTO (link-time optimization),
and then the graph will need more information.
DEV-13160
Some investigation into the disassembly of TAMER's binaries showed that Rust
was not able to conditionalize `expect`-like expressions as I was hoping due
to eager evaluation language semantics in combination with the use of
`format!`.
This solves the problem for the diagnostic system be creating types that
prevent this situation from occurring statically, without the need for a
lint.
This invokes clippy as part of `make check` now, which I had previously
avoided doing (I'll elaborate on that below).
This commit represents the changes needed to resolve all the warnings
presented by clippy. Many changes have been made where I find the lints to
be useful and agreeable, but there are a number of lints, rationalized in
`src/lib.rs`, where I found the lints to be disagreeable. I have provided
rationale, primarily for those wondering why I desire to deviate from the
default lints, though it does feel backward to rationalize why certain lints
ought to be applied (the reverse should be true).
With that said, this did catch some legitimage issues, and it was also
helpful in getting some older code up-to-date with new language additions
that perhaps I used in new code but hadn't gone back and updated old code
for. My goal was to get clippy working without errors so that, in the
future, when others get into TAMER and are still getting used to Rust,
clippy is able to help guide them in the right direction.
One of the reasons I went without clippy for so long (though I admittedly
forgot I wasn't using it for a period of time) was because there were a
number of suggestions that I found disagreeable, and I didn't take the time
to go through them and determine what I wanted to follow. Furthermore, it
was hard to make that judgment when I was new to the language and lacked
the necessary experience to do so.
One thing I would like to comment further on is the use of `format!` with
`expect`, which is also what the diagnostic system convenience methods
do (which clippy does not cover). Because of all the work I've done trying
to understand Rust and looking at disassemblies and seeing what it
optimizes, I falsely assumed that Rust would convert such things into
conditionals in my otherwise-pure code...but apparently that's not the case,
when `format!` is involved.
I noticed that, after making the suggested fix with `get_ident`, Rust
proceeded to then inline it into each call site and then apply further
optimizations. It was also previously invoking the thread lock (for the
interner) unconditionally and invoking the `Display` implementation. That
is not at all what I intended for, despite knowing the eager semantics of
function calls in Rust.
Anyway, possibly more to come on that, I'm just tired of typing and need to
move on. I'll be returning to investigate further diagnostic messages soon.
This introduces a number of abstractions, whose concepts are not fully
documented yet since I want to see how it evolves in practice first.
This introduces the concept of edge ontology (similar to a schema) using the
type system. Even though we are not able to determine what the graph will
look like statically---since that's determined by data fed to us at
runtime---we _can_ ensure that the code _producing_ the graph from those
data will produce a graph that adheres to its ontology.
Because of the typed `ObjectIndex`, we're also able to implement operations
that are specific to the type of object that we're operating on. Though,
since the type is not (yet?) stored on the edge itself, it is possible to
walk the graph without looking at node weights (the `ObjectContainer`) and
therefore avoid panics for invalid type assumptions, which is bad, but I
don't think that'll happen in practice, since we'll want to be resolving
nodes at some point. But I'll addres that more in the future.
Another thing to note is that walking edges is only done in tests right now,
and so there's no filtering or anything; once there are nodes (if there are
nodes) that allow for different outgoing edge types, we'll almost certainly
want filtering as well, rather than panicing. We'll also want to be able to
query for any object type, but filter only to what's permitted by the
ontology.
DEV-13160
Working with the graph can be confusing with all of the layers
involved. This begins to provide a better layer of abstraction that can
encapsulate the concept and enforce invariants.
Since I'm better able to enforce invariants now, this also removes the span
from the diagnostic message, since the invariant is now always enforced with
certainty. I'm not removing the runtime panic, though; we can revisit that
if future profiling shows that it makes a negative impact.
DEV-13160
This addresses the two outstanding `todo!` match arms representing errors in
lowering expressions into the graph. As noted in the comments, these errors
are unlikely to be hit when using TAME in the traditional way, since
e.g. XIR and NIR are going to catch the equivalent problems within their own
contexts (unbalanced tags and a valid expression grammar respectively).
_But_, the IR does need to stand on its own, and I further hope that some
tooling maybe can interact more directly with AIR in the future.
DEV-13160
This introduces a number of concepts together, again to demonstrate that
they were derived.
This introduces support for nested expressions, extending the previous
work. It also supports error recovery for dangling expressions.
The parser states are a mess; there is a lot of duplicate code here that
needs refactoring, but I wanted to commit this first at a known-good state
so that the diff will demonstrate the need for the change that will
follow; the opportunities for abstraction are plainly visible.
The immutable stack introduced here could be generalized, if needed, in the
future.
Another important note is that Rust optimizes away the `memcpy`s for the
stack that was introduced here. The initial Parser Context was introduced
because of `ArrayVec` inhibiting that elision, but Vec never had that
problem. In the future, I may choose to go back and remove ArrayVec, but I
had wanted to keep memory allocation out of the picture as much as possible
to make the disassembly and call graph easier to reason about and to have
confidence that optimizations were being performed as intended.
With that said---it _should_ be eliding in tamec, since we're not doing
anything meaningful yet with the graph. It does also elide in tameld, but
it's possible that Rust recognizes that those code paths are never taken
because tameld does nothing with expressions. So I'll have to monitor this
as I progress and adjust accordingly; it's possible a future commit will
call BS on everything I just said.
Of course, the counter-point to that is that Rust is optimizing them away
anyway, but Vec _does_ still require allocation; I was hoping to keep such
allocation at the fringes. But another counter-point is that it _still_ is
allocated at the fringe, when the context is initialized for the parser as
part of the lowering pipeline. But I didn't know how that would all come
together back then.
...alright, enough rambling.
DEV-13160
I had wanted to implement expression operations in terms of user-defined
functions (where primitives are just marked as intrinsic), and would still
like to, but I need to get this thing working, so I'll just include a note
for now.
Yes, TAMER's formalisms are inspired by APL, if that hasn't been documented
anywhere yet.
DEV-13160
This commit is purposefully coupled with changes that utilize it to
demonstrate that the need for this abstraction has been _derived_, not
forced; TAMER doesn't aim to be functional for the sake of it, since
idiomatic Rust achieves many of its benefits without the formalisms.
But, the formalisms do occasionally help, and this is one such
example. There is other existing code that can be refactored to take
advantage of this style as well.
I do _not_ wish to pull an existing functional dependency into TAMER; I want
to keep these abstractions light, and eliminate them as necessary, as Rust
continues to integrate new features into its core. I also want to be able
to modify the abstractions to suit our particular needs. (This is _not_ a
general recommendation; it's particular to TAMER and to my experience.)
This implementation of `Functor` is one such example. While it is modeled
after Haskell in that it provides `fmap`, the primitive here is instead
`map`, with `fmap` derived from it, since `map` allows for better use of
Rust idioms. Furthermore, it's polymorphic over _trait_ type parameters,
not method, allowing for separate trait impls for different container types,
which can in turn be inferred by Rust and allow for some very concise
mapping; this is particularly important for TAMER because of the disciplined
use of newtypes.
For example, `foo.overwrite(span)` and `foo.overwrite(name)` are both
self-documenting, and better alternatives than, say, `foo.map_span(|_|
span)` and `foo.map_symbol(|_| name)`; the latter are perfectly clear in
what they do, but lack a layer of abstraction, and are verbose. But the
clarity of the _new_ form does rely on either good naming conventions of
arguments, or explicit type annotations using turbofish notation if
necessary.
This will be implemented on core Rust types as appropriate and as
possible. At the time of writing, we do not yet have trait specialization,
and there's too many soundness issues for me to be comfortable enabling it,
so that limits that we can do with something like, say, a generic `Result`,
while also allowing for specialized implementations based on newtypes.
DEV-13160
Admittedly, there are _my_ debugging conventions. But I'm also the only one
working on this project right now.
I want to keep various things around without cluttering untracked file
output, because finding new files can be annoying in all the output.
Really, with a C background, I should have known that `write` may not write
all bytes, and I'm pretty sure I was aware, so I'm not sure how that slipped
my mind for every call. But it's not a great default, and I do feel like
`write_all` should be the deafult behavior, despite the syscall and C
library name.
It shouldn't take clippy to warn about something so significant.
This uses `ObjectIndex` to automatically narrow the type to what is
expected.
Given that `ObjectIndex` is supposed to mean that there must be an object
with that index, perhaps the next step is to remove the `Option` from `get`
as well.
DEV-13160
This makes the system a bit more ergonomic and introduces additional type
safety by associating the narrowed object type with the
`ObjectIndex` (previously `ObjectRef`). Not only does this allow us to
explicitly state the type of object wherever those indices are stored, but
it also allows the API to automatically narrow to that type when operating
on it again without the caller having to worry about it.
DEV-13160
This begins to place expressions on the graph---something that I've been
thinking about for a couple of years now, so it's interesting to finally be
doing it.
This is going to evolve; I want to get some things committed so that it's
clear how I'm moving forward. The ASG makes things a bit awkward for a
number of reasons:
1. I'm dealing with older code where I had a different model of doing
things;
2. It's mutable, rather than the mostly-functional lowering pipeline;
3. We're dealing with an aggregate ever-evolving blob of data (the graph)
rather than a stream of tokens; and
4. We don't have as many type guarantees.
I've shown with the lowering pipeline that I'm able to take a mutable
reference and convert it into something that's both functional and
performant, where I remove it from its container (an `Option`), create a new
version of it, and place it back. Rust is able to optimize away the memcpys
and such and just directly manipulate the underlying value, which is often a
register with all of the inlining.
_But_ this is a different scenario now. The lowering pipeline has a narrow
context. The graph has to keep hitting memory. So we'll see how this
goes. But it's most important to get this working and measure how it
performs; I'm not trying to prematurely optimize. My attempts right now are
for the way that I wish to develop.
Speaking to #4 above, it also sucks that I'm not able to type the
relationships between nodes on the graph. Rather, it's not that I _can't_,
but a project to created a typed graph library is beyond the scope of this
work and would take far too much time. I'll leave that to a personal,
non-work project. Instead, I'm going to have to narrow the type any time
the graph is accessed. And while that sucks, I'm going to do my best to
encapsulate those details to make it as seamless as possible API-wise. The
performance hit of performing the narrowing I'm hoping will be very small
relative to all the business logic going on (a single cache miss is bound to
be far more expensive than many narrowings which are just integer
comparisons and branching)...but we'll see. Introducing branching sucks,
but branch prediction is pretty damn good in modern CPUs.
DEV-13160
This will be used for expression start and end spans to merge into a span
that represents the entirety of the expression; see future commits for its
use.
Though, this has been generalized further than that to ensure that it makes
sense in any use case, to avoid potential pitfalls.
DEV-13160
This adds a line of padding between the last line of a source marking and
the first line of a footer, making it easier to read. This also matches the
behavior of Rust's error message.
This is something I intended to do previously, but didn't have the
time. Not that I do now, but now that we'll be showing some more robust
diagnostics to users, it ought to look decent.
DEV-13430
This moves the special handling of circular dependencies out of
`poc.rs`---and to be clear, everything needs to be moved out of there---and
into the source of the error. The diagnostic system did not exist at the
time.
This is one example of how easy it will be to create robust diagnostics once
we have the spans on the graph. Once the spans resolve to the proper source
locations rather than the `xmlo` file, it'll Just Work.
It is worth noting, though, that this detection and error will ultimately
need to be moved so that it can occur when performing other operation on the
graph during compilation, such as type inference and unification. I don't
expect to go out of my way to detect cycles, though, since the linker will.
DEV-13430
Previously this just exported the variable into the environment, but I'm not
comfortable with the lack of visibility that provides; I want to be able to
see not only that it's happening, which will help to debug issues, but also
when it's _not_ happening so that I know that it needs to be introduced into
a configuration at a particular installation site.
This ASG implementation is a refactored form of original code from the
proof-of-concept linker, which was well before the span and diagnostic
implementations, and well before I knew for certain how I was going to solve
that problem.
This was quite the pain in the ass, but introduces spans to the AIR tokens
and graph so that we always have useful diagnostic information. With that
said, there are some important things to note:
1. Linker spans will originate from the `xmlo` files until we persist
spans to those object files during `tamec`'s compilation. But it's
better than nothing.
2. Some additional refactoring is still needed for consistency, e.g. use
of `SPair`.
3. This is just a preliminary introduction. More refactoring will come as
tamec is continued.
DEV-13041
The previous commit had the ASG implicitly constructed and then
discarded. This will keep it around, which will be necessary not only for
imports, but for passing the ASG off to the next phases of lowering.
DEV-13429
This does not yet yield the produces ASG, but does set up the lowering
pipeline to prepare to produce it. It's also currently a no-op, with
`NirToAsg` just yielding `Incomplete`.
The goal is to begin to move toward vertical slices for TAMER as I start to
return to the previous approach of a handoff with the old compiler. Now
that I've gained clarity from my previous failed approach (which I
documented in previous commits), I feel that this is the best way forward
that will allow me to incrementally introduce more fine-grained performance
improvements, at the cost of some throwaway work as this progresses. But
the cost of delay with these build times is far greater.
DEV-13429
This finalizes the implementation for interpolation. There is some more
cleanup that can be done, but it is now functioning as intended and
providing errors.
Finally. How deeply exhausting all of this has been.
DEV-13156
This just cleans up these tests a bit before I add to them. What we're left
with follows the structure of most other parser tests and is atm a good
balance between boilerplate and clarity in isolation (a fair level of
abstraction).
Could possibly do better by putting the inner objects in a callback so that
the `Close` can be asserted on commonly as well, but that's a bit awkward
with how the assertion is based on the collection; we'd have to keep the
last item from being collected from the iterator. I'd rather not deal with
such restructuring right now and figuring out a decent pattern. Perhaps in
the future.
DEV-13156
This is the culmination of all the recent work---the third attempt at trying
to integrate this. It ended up much cleaner than what was originally going
to be done, but only after gutting portions of the system and changing my
approach to how NIR is parsed (WRT attributes). See prior commits for more
information.
The final step is to fill the error branches with actual errors rather than
`todo!`s.
What a relief.
DEV-13156
This begins to introduce the new, simplified NIR by creating tokens that
serve as the expansion for interpolation. Admittedly, `Text` may change, as
it doesn't really represent `<text>foo</text>`, and I'd rather that node
change as well, though I'll probably want to maintain some sort of BC.
DEV-13156
This removes quite a bit of work, and work that was difficult to reason
about. While I'm disappointed that that hard work is lost (aside from
digging it up in the commit history), I am happy that it was able to be
removed, because the extra complexity and cognitive burden was significant.
This removes more `memcpy`s than the sum state could have hoped to, since
aggregation is no longer necessary. Given that, there is a slight
performacne improvement. The re-introduction of required and duplicate
checks later on should be more efficient than this was, and so this should
be a net win overall in the end.
DEV-13346
This cleans up the old implementation now that it's no longer used (as of
the previous commit) by `ele_parse!`. It also removes the two error
variants that no longer apply: required attributes and duplicate
attributes.
DEV-13346
This handles the bulk of the integration of the new `attr_parse_stream!` as
a replacement for `attr_parse!`, which moves from aggregate attribute
objects to a stream of attribute-derived tokens. Rationale for this change
is in the preceding commit messages.
The first striking change here is how it affects the test cases: nearly all
`Incomplete`s are removed. Note that the parser has an existing
optimization whereby `Incomplete` with lookahead causes immediate recursion
within `Parser`, since those situations are used only for control flow and
to keep recursion out of `ParseState`s.
Next: this removes types from `nir::parse`'s grammar for attributes. The
types will instead be derived from NIR tokens later in the lowering
pipeline. This simplifies NIR considerably, since adding types into the mix
at this point was taking an already really complex lowering phase and making
it ever more difficult to reason about and get everything working together
the way that I needed.
Because of `attr_parse_stream!`, there are no more required attribute
checks. Those will be handled later in the lowering pipeline, if they're
actually needed in context, with possibly one exception: namespace
declarations. Those are really part of the document and they ought to be
handled _earlier_ in the pipeline; I'll do that at some point. It's not
required for compilation; it's just required to maintain compliance with the
XML spec.
We also lose checks for duplicate attributes. This is also something that
ought to be handled at the document level, and so earlier in the pipeline,
since XML cares, not us---if we get a duplicate attribute that results in an
extra NIR token, then the next parser will error out, since it has to check
for those things anyway.
A bunch of cleanup and simplification is still needed; I want to get the
initial integration committed first. It's a shame I'm getting rid of so
much work, but this is the right approach, and results in a much simpler
system.
DEV-13346
This really does need documentation.
With that said, this changes things up a bit: the value is now derived from
an `SPair` rather than an `Attr`, given that the name is redundant. We do
not need the attribute name span, since the philosophy is that we're
stripping the document and it should no longer be important beyond the
current context.
It does call into question errors, but my intent in the future is to be able
to have the lowering pipline augment errors with its current state---since
we're streaming, then an error that is encountered during lowering of an
element will still have the element parser in the state representing the
parsing of that element; so that information does not need to be propagated
down the pipeline, but can be augmented as it bubbles back up.
More on that at some point in the future; not right now.
DEV-13346
As I talked about in the previous commit, this is going to be the
replacement for the aggreagte `attr_parse!`; the next commit will integrate
it into `ele_parse!` so that I can begin to remove the old one.
It is disappointing, since I did put a bit of work into this and I think the
end result was pretty neat, even if was never fully utilized. But, this
simplifies things significantly; no use in maintaining features that serve
no purpose but to confound people.
DEV-13346
Alright, this has been a rather tortured experience. The previous commit
began to state what is going on.
This is reversing a lot of prior work, with the benefit of
hindsight. Little bit of history, for the people who will probably never
read this, but who knows:
As noted at the top of NIR, I've long wanted a very simple set of general
primitives where all desugaring is done by the template system---TAME is a
metalanguage after all. Therefore, I never intended on having any explicit
desugaring operations.
But I didn't have time to augment the template system to support parsing on
attribute strings (nor am I sure if I want to do such a thing), so it became
clear that interpolation would be a pass in the compiler. Which led me to
the idea of a desugaring pass.
That in turn spiraled into representing the status of whether NIR was
desugared, and separating primitives, etc, which lead to a lot of additional
complexity. The idea was to have a Sugared and Plan NIR, and further within
them have symbols that have latent types---if they require interpolation,
then those types would be deferred until after template expansion.
The obvious problem there is that now:
1. NIR has the complexity of various types; and
2. Types were tightly coupled with NIR and how it was defined in terms of
XML destructuring.
The first attempt at this didn't go well: it was clear that the symbol types
would make mapping from Sugared to Plain NIR very complicated. Further,
since NIR had any number of symbols per Sugared NIR token, interpolation was
a pain in the ass.
So that lead to the idea of interpolating at the _attribute_ level. That
seemed to be going well at first, until I realized that the token stream of
the attribute parser does not match that of the element parser, and so that
general solution fell apart. It wouldn't have been great anyway, since then
interpolation was _also_ coupled to the destructuring of the document.
Another goal of mine has been to decouple TAME from XML. Not because I want
to move away from XML (if I did, I'd want S-expressions, not YAML, but I
don't think the team would go for that). This decoupling would allow the
use of a subset of the syntax of TAME in other places, like CSVMs and YAML
test cases, for example, if appropriate.
This approach makes sense: the grammar of TAME isn't XML, it's _embedded
within_ XML. The XML layer has to be stripped to expose it.
And so that's what NIR is now evolving into---the stripped, bare
repsentation of TAME's language. That also has other benefits too down the
line, like a REPL where you can use any number of syntaxes. I intend for
NIR to be stack-based, which I'd find to be intuitive for manipulating and
querying packages, but it could have any number of grammars, including
Prolog-like for expressing Horn clauses and querying with a
Prolog/Datalog-like syntax. But that's for the future...
The next issue is that of attribute types. If we have a better language for
NIR, then the types can be associated with the NIR tokens, rather than
having to associate each symbol with raw type data, which doesn't make a
whole lot of sense. That also allows for AIR to better infer types and
determine what they ought to be, and further makes checking types after
template application natural, since it's not part of NIR at all. It also
means the template system can naturally apply to any sources.
Now, if we take that final step further, and make attributes streaming
instead of aggregating, we're back to a streaming pipeline where all
aggregation takes place on the ASG (which also resolves the memcpy concerns
worked around previously, also further simplifying `ele_parse` again, though
it sucks that I wasted that time). And, without the symbol types getting
in the way, since now NIR has types more fundamentally associated with
tokens, we're able to interpolate on a token stream using simple SPairs,
like I always hoped (and reverted back to in the previous commit).
Oh, and what about that desugaring pass? There's the issue of how to
represent such a thing in the type system---ideally we'd know statically
that desugaring always lowers into a more primitive NIR that reduces the
mapping that needs to be done to AIR. But that adds complexity, as
mentioned above. The alternative is to just use the templat system, as I
originally wanted to, and resolve shortcomings by augmenting the template
system to be able to handle it. That not only keeps NIR and the compiler
much simpler, but exposes more powerful tools to developers via TAME's
metalanguage, if such a thing is appropriate.
Anyway, this creates a system that's far more intuitive, and far
simpler. It does kick the can to AIR, but that's okay, since it's also
better positioned to deal with it.
Everything I wrote above is a thought dump and has not been proof-read, so
good luck! And lets hope this finally works out...it's actually feeling
good this time. The journey was necessary to discover and justify what came
out of it---everything I'm stripping away was like a cocoon, and within it
is a more beautiful and more elegant TAME.
DEV-13346
Also: Revert "tamer: nir::desugar::interp: Token {SPair=>Attr}"
This reverts commit 7fd60d6cdafaedc19642a3f10dfddfa7c7ae8f53.
This reverts commit 12a008c66414c3d628097e503a98c80687e3c088.
This has been quite a tortured experience, trying to figure out how to best
fit desugaring into the existing system. The truth is that it ultimately
failed because I was not sticking with my intuition---I was trying to get
things out quickly by compromising on the design, and in the end, it saved
me nothing.
But I wouldn't say that it was a waste of time---the path was a dead end,
but it was full of experiences.
More to come, but interpolation is back to operating on NIR directly, and I
chose to treat it as a source-to-source mapping and not represent it using
the type system---interpolation can be an optional feature when writing TAME
frontends (the principal one being the XML-based one), and it's up to later
checks to assert that identifiers match a given domain.
I am disappointed by the additional context we lose here, but that can
always be introduced in the future differently, e.g. by maintaining a
dictionary of additional context for spans that can be later referenced for
diagnostic purposes. But let's worry about that in the future; it doesn't
make sense to further complicate IRs for such a thing.
DEV-13346
Converts to use TAME's diagnostic panics, same as previous commits. Also
introduces impl for `Result`, which I apparently hadn't needed yet.
In the future, I hope trait impl specializations will be available to
automatically derive and expose span information in these diagnostic
messages for certain types.
DEV-13156
This changes the input token from a more generic `SPair` to `Attr`, which
reflects the new target integration point in the `attr_parse!`
parser-generator.
This is a compromise---I'd like for it to remain generic and have stitching
deal with all integration concerns, but I have spent far too much time on
this and need to keep moving.
With that said, we do benefit from knowing where this must fit in---it's
easier to reason about in a more concrete way, and we can take advantage of
the extra information rather than being burdened by its presence and
ignoring it. We need to be able to convert back into `XirfToken` (see a
recent commit that discusses that) for `StitchExpansion`, which is why
`Attr` is here. And since it is, we can use it to explain to the user not
just the interpolation specification used to derive params, but also the
attribute it is associated with. This is what TAME (in XSLT) does today,
IIRC (I wrote it, I just forget exactly). It also means that I can name the
parameters after the attribute.
So, that'll be in a following commit; I was disappointed when my prior
approach with `SPair` didn't give me enough information to be able to do
that, since I think it's important that the system be as descriptive as
possible in how it derives information. Of course, traces would reveal how
the parser came about the derivation, but that requires recompilation in a
special tracing mode.
DEV-13156
Of course I would run into integration issues. My foresight is lacking.
The purpose of this is to allow for type narrowing before passing data to a
more specialized ParseState, so that the other ParseState doesn't need to
concern itself with the entire domain of inputs that it doesn't need, and
repeat unnecessary narrowing.
For example, consider XIRF: it has an `Attr` variant, which holds an `Attr`
object. We'll want to desugar that object. It does not make sense to
require that the desugaring process accept `XirfToken` when we've already
narrowed it to an `Attr`---we should accept an Attr.
However, we run into a problem immediately: what happens with tokens that
bubble back up due to lookahead or errors? Those tokens need to be
converted _back_ (widened). Fortunately, widening is a much easier process
than narrowing---we can simply use `From`, as we do today so many other
places.
So, this still keeps the onus of narrowing on the caller, but for now that
seems most appropriate. I suspect Rust would optimize away duplicate
checks, but that still leaves the maintenance concern---the two narrowings
could get out of sync, and that's not acceptable.
Unfortunately, this is just one of the problems with integration...
DEV-13156
My initial plan with expansion was to wrap a `PasteState` in another that
unwraps `Expansion` and converts into a `Dead` state, so that existing
`TransitionResult` stitching methods (`delegate`, specifically) could be
used.
But the desire to use that existing method was primarily because stitching
was a complex operation that was abstracted away _as part of the `delegate`
method_, which made writing new ones verbose and difficult. Thus began the
previous commits to begin to move that responsibility elsewhere so that it
could be more composable.
This continues with that, introducing a new trait that will culminate in the
removal of a wrapping `ParseState` in favor of a stitching method. The old
`StitchableExpansionState` is still used for tests, which demonstrates that
the boilerplate problem still exists despite improvements made here These
will become more generalized in the future as I have time (and the
functional aspects of the code more formalized too, now that they're taking
shape).
The benefit of this is that we avoid having to warp our abstractions in ways
that don't make sense (use of a dead state transition) just to satisfy
existing APIs. It also means that we do not need the boilerplate of a
`ParseState` any time we want to introduce this type of
stitching/delegation. It also means that those methods can eventually be
extracted into more general traits in the future as well.
Ultimately, though, the two would have accomplished the same thing. But the
difference is most emphasized in the _parent_---the actual stitching still
has to take place for desugaring in the attribute parser, and I'd like for
that abstraction to still be in terms of expansion. But if I utilized
`StitchableExpansionState`, which converted into a dead state, I'd have to
either forego the expansion abstraction---which would make the parser even
more confusing---or I'd have to create _another_ abstraction around the dead
state, which would mean that I stripped one abstraction just to introduce
another one that's essentially the same thing. It didn't feel right, but it
would have worked.
The use of `PhantomData` in `StitchableExpansionState` was also a sign that
something wasn't quite right, in terms of how the abstractions were
integrating with one-another.
And so here we are, as I struggle to wade my way through all of the yak
shavings and make any meaningful progress on this project, while others
continue to suffer due to slow build times.
I'm sorry. Even if the system is improving.
DEV-13156
This is just intended to simplify the job of panicing when something is
expected to be `None`. In my case, `Lookahead`; see upcoming commits.
This is intended to be generalized to more than just `Option`, but I have no
use for it elsewhere yet; I primarily just needed to implement a method on
`Option` so that I could have the ergonomics of the dot notation.
DEV-13156
There's no use in duplicating this in util::expand.
Lookahead tokens are one of the few invariants that I haven't taken the time
of enforcing using the type system, because it'd be quite a bit of work that
I do not have time for, and may not be worth it with changes that may make
the system less ergonomic. Nonetheless, I do hope to address it at some
point in the (possibly-far) future.
If ever you encounter this diagnostic message, ask yourself how stable TAMER
otherwise is and how many other issues like this have been entirely
prevented through compile-time proofs using the type system.
DEV-13156
As in previous commits, this continues to replace panics with
`diagnostic_panic!`, which provides much more useful information both for
debugging and to help the user possibly work around the problem. And lets
the user know that it's not their fault, and it's a TAMER bug that should be
reported.
...am I going to rationalize it in each commit message?
DEV-13156
This moves enough of the handling of complex type conversions into the
various components of `TransitionResult` (and itself), which simplifies
delegation and opens up the possibility of having specialized
delegation/stitching methods implemented atop of `TransitionResult`.
DEV-13156
These delegation methods have been a pain in my ass for quite some time, and
their lack of generalization makes the introduction of new delegation
methods (in the general sense, not necessarily trait methods) very tedious
and prone to inconsistencies.
I'm going to progressively refactor them in separate commits so it's clear
what I'm doing, primarily for future me to reference if need be.
DEV-13156
This beings to introduce more primitive operations to `TransitionResult` and
its components so that I can actually work with them without having to write
a bunch of concrete, boilerplate implementations. This is demonstrated in
part by `EchoState` (which is nearly all boilerplate, but whose correctness
should be verifiable at a glance), which will be used going forward as a
basis for default implementations for parsers (e.g. expansion delegation).
DEV-13156
This has evolved into a more robust and independent concept, but it is still
a utility in the sense that it's utilizing existing parsing framework
features and making them more convenient.
DEV-13156
These traits serve to abstract away some of the type-level details and
clearly state what the end result is (something stitchable with a parent).
I'm admittedly battling myself on this concept a bit. The proper layer of
abstraction is the concept of expansion, which is an abstraction that is
likely to be maintained all the way through, but we strip the abstraction
for the sake of delegation. Maybe the better option is to provide a
different method of delegation and avoid the stripping at all, and avoid the
awkward interaction with the dead state.
The awkwardness comes from the fact that delegating right now is so rigid
and defined in terms of a method on state rather than a mapping between
`TransitionResult`s. But I really need to move on... ;_;
The original design was trying to generalize this such that composition at
the attribute parser level (for NIR) would be able to just accept any
sitchable parser with the convention that the dead state is the replacement
token. But that is the wrong layer of abstraction, which not only makes it
confusing, but is asking for trouble when someone inevitably violates that
contract.
With all of that said, `StitchableExpansionState` _is_ a delegation. It
could just as easily be a function (`is_accepting` always delegates too), so
perhaps that should just be generalized as reifying delegation as a
`ParseState`.
DEV-13156
This parser really just allows me to continue developing the NIR
interpolation system using `Expansion` terminology, and avoid having to use
dead states in tests. This allows for the appropriate level of abstraction
to be used in isolation, and then only be stripped when stitching is
necessary.
Future commits will show how this is actually integrated and may introduce
additional abstraction to help.
DEV-13156
This is a shift in approach.
My original idea was to try to keep NIR parsing the way it was, since it's
already hard enough to reason about with the `ele_parse!` parser-generator
macro mess. The idea was to produce an IR that would explicitly be denoted
as "maybe sugared", and have a desugaring operation as part of the lowering
pipeline that would perform interpolation and lower the symbol into a plain
version.
The problem with that is:
1. The use of the type was going to introduce a lot of mapping for all the
NIR token variants there are going to be; and
2. _The types weren't even utilized for interpolation._
Instead, if we interpolated _as attributes are encountered_ while parsing
NIR, then we'd be able to expand directly into that NIR token stream and
handle _all_ symbols in a generic way, without any mapping beyond the
definition of NIR's grammar using `ele_parse!`.
This is a step in that direction---it removes `NirSymbolTy` and introduces a
generic abstraction for the concept of expansion, which will be utilized
soon by the attribute parser to allow replacing `TryFrom` with something
akin to `ParseFrom`, or something like that, which is able to produce a
token stream before finally yielding the value of the attribute (which will
be either the original symbol or the replacement metavariable, in the case
of interpolation).
(Note that interpolation isn't yet finished---errors still need to be
implemented. But I want a working vertical slice first.)
DEV-13156
This was a substantial change. Design and rationale are documented on
`AttrFieldSum` and related as part of this change, so please review the diff
for more information there.
If you're a Ryan employee, DEV-13209 gives plenty of profiling information,
including raw data and visualizations from kcachegrind. For everyone else:
you're able to easy produce your own from this commit and the previous and
comparing the `__memcpy_avk_unaligned_erms` calls. The reduction is
significant in this commit (~90%), and the number of Parsers invoking it has
been reduced. Rust has been able to optimize more aggressively, and
compound some of those optimizations, with the smaller `NirParseState`
width.
It also worth noting that `malloc` calls do not change at all between
these two changes, so when we refer to memory, we're referring to
pre-allocated memory on the stack, as TAMER was designed to utilize.
DEV-13209
This is a diagnostic replacement for `unreachable!`.
Eventually TAMER'll have build-time checks to enforce the use of these over
alternatives; I need to survey the old instances on a case-by-case basis to
see what diagnostic information can be reasonably presented in that context.
DEV-13209
The spans were previously not being calculated relative to the offset of the
original symbol span. Tests were passing because all of those spans began
at offset 0.
DEV-13156
This demonstrates how desugaring of interpolated strings will work, testing
one of the happy paths. The remaining work to be done is largely
refactoring; handling some other cases; and errors. Each of those items are
marked with `todo!`s.
I'm pleased with how this is turning out, and I'm excited to see diagnostic
reporting within the specification string using the derived spans once I get
a bit further along; this robust system is going to be much more helpful to
developers than the existing system in XSLT.
This also eliminates the ~50% performance degredation mentioned in a recent
commit by eliminating the SugaredNirSymbol enum and replacing it with a
newtype; this is a much better approach, though it doesn't change that I do
need to eventually address the excessive `memcpy`s on hot code paths.
DEV-13156
Not sure why I didn't add a prelude sooner, considering all the import
boilerplate. This will evolve as needed and I'll go back and replace other
imports when I'm not in the middle of something.
DEV-13156
Add initial descriptions and consolodate some of the types. There'll be
more to come; this is just to get `Display` derives working for types
that'll be using it. I'd like to see where this description manifests
itself before I decide how user-friendly I'd like it to be.
DEV-13156
This mirror is only a `Todo` variant at the moment, but my hope had been to
try to creatively nest or use generics to simplify the conversaion between
the two flavors without a lot of boilerplate. But it doesn't seem like I'm
going to be successful, and may have to resort to macros to remove
boilerplate.
But I need to stop fighting with myself and move on. Though I would still
like to keep the types purely compile-time via const generics if possible,
since they're not needed in memory (or disk) until we get to templates;
they're otherwise static relative to a NIR token variant.
DEV-13209
This simply detects whether a value will need to be further parsed for
interpolation; it does not yet perform the parsing itself, which will happen
during desugaring.
This introduces a performance regression, for an interesting reason. I
found that introducing a single new variant to `SugaredNir` (with a
`(SymbolId, Span)` pair), was causing the width of the `NirParseState` type
to increase just enough to cause Rust to be unable to optimize away a
significant number of memcpys related to `Parser` moves, and consequently
reducing performance by nearly 50% for `tamec`. Yikes.
I suspected this would be a problem, and indeed have tried in all other
cases to avoid aggregation until the ASG---the problem is that I had wanted
to aggregate attributes for NIR so that the IR could actually make some
progress toward simplifying the stream (and therefore working with the
data), and be able to validate against a grammar defined in a single
place. The problem is that the `NirParseState` type contains a sum type for
every attribute parser, and is therefore as wide as the largest one. That
is what Rust is having trouble optimizing memcpy away for.
Indeed, reducing the number of attributes improves the situation
drastically. However, it doesn't make it go away entirely.
If you look at a callgrind profile for `tameld` (or a dissassembly), you'll
notice that I put quite a bit of effort into ensuring that the hot code path
for the lowering pipeline contains _no_ memcpys for the parsers. But that
is not the case with `tamec`---I had to move on. But I do still have the
same escape hatch that I introduced for `tameld`, which is the mutable
`Context`.
It seems that may be the solution there too, but I want to get a bit further
along first to see how these data end up propagating before I go through
that somewhat significant effort.
DEV-13156
Various parts of the system have to be converted to use `diagnostic_panic!`,
which makes it very clear that this is a bug in TAMER that should be
reported. I just happened to see this one near code I was about to touch.
DEV-13156
This introduces the concept of sugared NIR and provides the boilerplate for
a desugaring pass. The earlier commits dealing with cleaning up the
lowering pipeline were to support this work, in particular to ensure that
reporting and recovery properly applied to this lowering operation without
adding a ton more boilerplate.
DEV-13158
I'm struggling to go much further yet without sorting out some other things
first with regards to mutable `Context` and, in particular, the ASG.
I'm going to pause on refactoring the lowering pipeline---it's been improved
significantly with the recent work---and I will continue in the next few
weeks.
DEV-13158
Lowering errors in tamec end up utilizing recovery and reporting, so there
is a distinction between recoverable and unrecoverable errors.
tameld aborts on the first error, since recovery is not currently
supported (we'll want to add it, since tameld should output e.g. lists of
unresolved externs).
Note that tamec does not yet handle `FinalizeError` like tameld because it
uses `Lower::lower`, which does not yet finalize (though it does in practice
when it reaches the end of the stream and auto-finalizes, but that is
widened into a `ParseError`).
DEV-13158
This helps to clarify the situations under which these errors can occur, and
the generality also helps to show why the inner types are as they
are (e.g. use of `String`).
But more importantly, this allows for an error type in `finalize` that is
detached from the `ParseState`, which will be able to be utilized in the
lowering pipeline as a more general error distinguishable from other
lowering errors. At the moment I'm maintaining BC, but a following commit
will demonstrate the use case to introduce recoverable vs. non-recoverable
errors.
DEV-13158
This newtype allows a caller to prove (using types) that a parser of a given
type (`ParseState`) has been finalized.
This will be used by the lowering pipeline to ensure that all parsers in the
pipeline end up getting finalized (as you can see from a TODO added in the
code, one of them is missing). The lack of such a type was an oversight
during the (rather stressed) development of the parsing system, and I
shouldn't need to resort to unit tests to verify that parsers have been
finalized.
DEV-13158
This reverts commit 85ec626fcd804eb2fac3fd6f0339182554f72cfd.
This revert had to be modified to work alongside other changes. Interior
mutability is fortunately no longer needed after the previous commit which
allows reporting to occur in a single place in the lowering pipeline (at the
terminal parser).
DEV-13158
The term "terminal parser" isn't formalized yet in the system, but is meant
to refer to the innermost parser that is responsible for pulling tokens
through the lowering pipeline.
This approach is more of what one would expect when dealing with
`Result`-like monads---we are effectively chaining the inner operation while
propagating errors to short-circuit lowering and let the caller decide
whether recovery ought to be permitted with diagnostic messages. This will
become more clear as it is further refactored.
This also means that the previous changes for introducing interior
mutability for a shared mutable `Reporter` can be reverted, which is great,
since that approach was antithetical to how the streaming pipeline
operates (and introduces awkward mutable state into an
otherwise-mostly-immutable system).
DEV-13158
This extracts error tracking into the Reporter itself, which is already
shared between lowering operations. This can then be used to display the
number of errors.
A new formatter (in tamer::fmt) will be added to handle the singular/plural
conversion in place of "error(s)" in the future; I have more important
things to work on right now.
DEV-13158
Previously these errors would immediately abort.
This results in some duplicate code, but it's beginning to derive a common
implementation. Check out the commits that follow; this is really an
intermediate refactoring state.
DEV-13158
Another baby step. The small commits are intended to allow comprehension of
what changes when looking at the diffs.
This also removes a comment stating that errors do not fail compilation,
since they most certainly do.
DEV-13158
This begins refactoring the lowering pipeline to begin to obviate
abstraction boundaries. The lowering pipeline is the backbone of the
system, and so it needs to become clear and self-documenting, which will
take a little bit of work.
DEV-13158
This always annoys me when I add a dependency and I don't know where I ought
to put it.
Anyway, I was originally going to add the `regex` crate, but with further
planning, I may not end up having use for it. Nonetheless, at least this is
consistent.
Just preparing to actually define NIR itself. The _grammar_ has been
represented (derived from our internal systems, using them as a test case),
but the IR itself has not yet received a definition.
DEV-7145
This is a quick-and-dirty change. The lowering pipeline needs a proper
abstraction, but I'm about to be on vacation at the end of the week and
would like to get NIR->AIR lowering started before I consider that
abstraction further, so this will do for now.
NIR parsing has been tested in production without failing for over a week.
DEV-7145
This was originally the "noramlized" IR, but that's not possible to do
without template expansion, which is going to happen at a later point. So,
this is just "NIR", pronounced "near", which is an IR that is "near" to the
source code. You can define it was "Near IR" if you want, but it's just a
homonym with a not-quite-defined acronym to me.
DEV-7145
A type alias was added for BC before errors were hoisted out in a previous
commit, but they are unnecessary because of the associated type on
`ParseState`.
This also corrects the long-existing issue of using generated identifiers in
tests.
DEV-7145
This moves `paste::paste!` up a line and reduces a level of indentation,
since it's so squished. Aside from docblock reformatting, there are no
other changes.
DEV-7145
This slims out the macro even further. It does result in an
awkwardly-placed `PhantomData` because I don't want to add another variant
that isn't actually used (since they represent states).
DEV-7145
This is in preparation for hoisting out the common states, as was done with
the Sum NT in a previous commit.
I also think that organizing states in this way is more clear. The previous
embedding of the variants named after the NTs themselves was because the
parser was storing the child state within it, before the introduction of the
superstate trampoline.
DEV-7145
Everything except for one state was already accounted for. We can now have
confidence that the parser will never panic due to state transitions (beyond
legitimate error conditions).
There are some `unreachable!`s to contend with still.
DEV-7145
This is the same as the previous commits, but for non-sum NTs.
This also extracts errors into a separate module, which I had hoped to do in
a separate commit, but it's not worth separating them. My _original_ reason
for doing so was debugging (I'll get into that below), but I had wanted to
trim down `ele.rs` anyway, since that mess is large and a lot to grok.
My debugging was trying to figure out why Rust was failing to derive
`PartialEq` on `NtError` because of `AttrParseError`. As it turns out,
`AttrParseError::InvalidValue` was failing, thus the introduction of the
`PartialEq` trait bound on `AttrParseState::ValueError`. Figuring this out
required implementing `PartialEq` myself without `derive` (well, using LSP,
which did all the work for me).
I'm not sure why this was not failing previously, which is a bit of a
concern, though perhaps in the context of the macro-expanded code, Rust was
able to properly resolve the types.
DEV-7145
The `ele_parse!` macro is a monstrosity, and expands into many different
identifiers. The hope is that chipping away at things like this will not
only make the template easier to understand by framing portions of the
problem in terms of more traditional Rust code, but will also hopefully
reduce compile times by reducing the amount of code that is expanded by the
macro.
DEV-7145
This introduces NIR, but only as an accepting grammar; it doesn't yet emit
the NIR IR, beyond TODOs.
This modifies `tamec` to, while copying XIR, also attempt to lower NIR to
produce parser errors, if any. It does not yet fail compilation, as I just
want to be cautious and observe that everything's working properly for a
little while as people use it, before I potentially break builds.
This is the culmination of months of supporting effort. The NIR grammar is
derived from our existing TAME sources internally, which I use for now as a
test case until I introduce test cases directly into TAMER later on (I'd do
it now, if I hadn't spent so much time on this; I'll start introducing tests
as I begin emitting NIR tokens). This is capable of fully parsing our
largest system with >900 packages, as well as `core`.
`tamec`'s lowering is a mess; that'll be cleaned up in future commits. The
same can be said about `tameld`.
NIR's grammar has some initial documentation, but this will improve over
time as well.
The generated docs still need some improvement, too, especially with
generated identifiers; I just want to get this out here for testing.
DEV-7145
This includes when on the last state / expecting a close.
Previously, there were a couple major issues:
1. After parsing an NT, we can't allow preemption because we must emit a
dead state so that we can remove the NT from the stack, otherwise
they'll never close (until the parent does) and that results in
unbounded stack growth for a lot of siblings. Therefore, we cannot
preempt on `Text`, which causes the NT to receive it, emit a dead
state, transition away from the NT, and not accept another NT of the
same type after `Text`.
2. When encountering an unknown element, the error message stated that a
closing tag was expected rather than one of the elements accepted by the
final NT.
For #1, this was solved by allowing the parent to transition back to the NT
if it would have been matched by the previous NT. A future change may
therefore allow us to remove repetition handling entirely and allow the
parent to deal with it (maybe).
For #2, the trouble is with the parser generator macro---we don't have a
good way of knowing the last NT, and the last NT may not even exist if none
was provided. This solution is a compromise, after having tried and failed
at many others; I desperately need to move on, and this results in the
correct behavior and doesn't sacrifice performance. But it can be done
better in the future.
It's also worth noting for #2 that the behavior isn't _entirely_ desirable,
but in practice it is mostly correct. Specifically, if we encounter an
unknown token, we're going to blow through all NTs until the last one, which
will be forced to handle it. After that, we cannot return to a previous NT,
and so we've forefitted the ability to parse anything that came before it.
NIR's grammar is such that sequences are rare and, if present, there's
really only ever two NTs, and so this awkward behavior will rarely cause
practical issues. With that said, it ought to be improved in the future,
but let's wait to see if other parts of the lowering pipeline provide more
appropriate places to handle some of these things (even though it really
ought to be handled at the grammar level).
But I'm well out of time to spend on this. I have to move on.
DEV-7145
Mike Gerwitz
Brandon Ellis