Now that the feature flag for the parser is a command line option, it is
useful to be able to run it on any package and see what errors arise, to use
as a guide for development with the goal of getting a particular package to
compile.
This converts the TODO panic into a recoverable error so that the parser can
spit out as many errors as it can.
DEV-13162
This provides some initial information to help guide a user to discover how
TAMER works, though either the source code or the generated
documentation. This will improve over time, since all of the high-level
abstractions are still under development.
DEV-13162
This introduces `xmlo-experimental` for `--emit`, allowing the new parser to
be toggled selectively for individual packages. This has a few notable
benefits:
1. We'll be able to conditionally compile packages as they are
supported (TAMER will target specific packages in our system to try to
achieve certain results more quickly);
2. This cleans up the code a bit by removing awkward gated logic, allowing
natural abstractions to form; and
3. Removing the compile-time feature flag ensures that the new features
are always built and tested; there are fewer configuration combinations
to test.
DEV-13162
This flag should have never been sprinkled here; it makes the system much
harder to understand.
But, this is working toward a command-line tamec option to toggle NIR
lowering on/off for various packages.
DEV-13162
This was a significant undertaking, with a few thrown-out approaches. The
documentation describes what approach was taken, but I'd also like to
provide some insight into the approaches that I rejected for various
reasons, or because they simply didn't work.
The problem that this commit tries to solve is encapsulation of error
types.
Prior to the introduction of the lowering pipeline macro
`lower_pipeline!`, all pipelines were written by hand using `Lower` and
specifying the applicable types. This included creating sum types to
accommodate each of the errors so that `Lower` could widen automatically.
The introduction of the `lower_pipeline!` macro resolved the boilerplate and
type complexity concerns for the parsers by allowing the pipeline to be
concisely declared. However, it still accepted an error sum type `ER` for
recoverable errors, which meant that we had to break a level of
encapsulation, peering into the pipeline to know both what parsers were in
play and what their error types were.
These error sum types were also the source of a lot of tedious boilerplate
that made adding new parsers to the pipeline unnecessarily unpleasant;
the purpose of the macro is to make composition both easy and clear, and
error types were undermining it.
Another benefit of sum types per pipeline is that callers need only
aggregate those pipeline types, if they care about them, rather than every
error type used as a component of the pipeline.
So, this commit generates the error types. Doing so was non-trivial.
Associated Types and Lifetimes
------------------------------
Error types are associated with their `ParseState` as
`ParseState::Error`. As described in this commit, TAMER's approach to
errors is that they never contain non-static lifetimes; interning and
copying are used to that effect. And, indeed, no errors in TAMER have
lifetimes.
But, some `ParseState`s may. In this case, `AsgTreeToXirf`:
```
impl<'a> ParseState for AsgTreeToXirf<'a> {
// [...]
type Error = AsgTreeToXirfError;
// [...]
}
```
Even though `AsgTreeToXirfError` does not have a lifetime, the `ParseState`
it is associated with _does_`. So to reference that type, we must use
`<AsgTreeToXirf<'a> as ParseState>::Error`. So if we have a sum type:
```
enum Sum<'a> {
// ^^ oh no! vv
AsgTreeToXirfError(<AsgTreeToXirf<'a> as ParseState>::Error),
}
```
There's no way to elide or make anonymous that lifetime, since it's not
used, at the time of writing. `for<'a>` also cannot be used in this
context.
The solution in this commit is to use a macro (`lower_error_sum`) to rewrite
lifetimes: to `'static`:
```
enum Sum {
AsgTreeToXirfError(<AsgTreeToXirf<'static> as ParseState>::Error),
}
```
The `Error` associated type will resolve to `AsgTreeToXirfError` all the
same either way, since it has no lifetimes of its own, letalone any
referencing trait bounds.
That's not to say that we _couldn't_ support lifetimes as long as they're
attached to context, but we have no need to at the moment, and it adds
significant cognitive overhead. Further, the diagnostic system doesn't deal
in lifetimes, and so would need reworking as well. Not worth it.
An alternative solution to this that was rejected is an explicitly `Error`
type in the macro application:
```
// in the lowering pipeline
|> AsgTreeToXirf<'a> { // lifetime
type Error = AsgTreeToXirfError; // no lifetime
}
```
But this requires peeling back the `ParseState` to see what its error is and
_duplicate_ it here. Silly, and it breaks encapsulation, since the lowering
pipeline is supposed to return its own error type.
Yet another option considered was to standardize a submodule convention
whereby each `ParseState` would have a module exporting `Error`, among other
types. This would decouple it from the parent type. However, we still have
the duplication between that and an associated type. Further, there's no
way to enforce this convention (effectively a module API)---the macro would
just fail in obscure ways, at least with `macro_rules!`. It would have been
an ugly kluge.
Overlapping Error Types
-----------------------
Another concern with generating the sum type, resolved in a previous commit,
was overlapping error types, which prohibited `impl From<E> for ER`
generation.
The problem with that a number of `ParseState`s used `Infallible` as their
`Error` type. This was resolved in a previous commit by creating
Infallible-like newtypes (variantless enums).
This was not the only option. `From` fits naturally into how TAMER handles
sum types, and fits naturally into `Lower`'s `WidenedError`. The
alternative is generating explicit `map_err`s in `lower_pipeline!`. This
would have allowed for overlapping error types because the _caller_ knows
what the correct target variant is in the sum type.
The problem with an explicit `map_err` is that it places more power in
`lower_pipeline!`, which is _supposed_ to be a macro that simply removes
boilerplate; it's not supposed to increase expressiveness. It's also not
fun dealing with complexity in macros; they're much more confusing that
normal code.
With the decided-upon approach (newtypes + `From`), hand-written `Lower`
pipelines are just as expressive---just more verbose---as `lower_pipeline!`,
and handles widening for you. Rust's type system will also handle the
complexity of widening automatically for us without us having to reason
about it in the macro. This is not always desirable, but in this case, I
feel that it is.
This configures the pipeline and returns a closure that can then be provided
with the source and sink.
The next obvious step would be to curry the source and sink.
But I wanted to commit this before I take a different (but equivalent)
approach that makes the pipeline operations more explicit and helps to guide
the user (developer) in developing and composing them. The FP approach is
less boilerplate, but is also more general and provides less
guidance. Given that composition at the topmost levels of the system,
especially with all the types involved, is one of the most confusing aspects
of the system---and one of the most important to get right and make clear,
since it's intended to elucidate the entire system at a high level, and
guide the reader. Well, it does a poor job at that now, but that's the
ultimate goal.
In essence---brutally general abstractions make sense at lower levels, but
the complexity at higher levels benefits from rigid guardrails, even though
it does not necessitate it.
DEV-13162
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
More information will be presented in the commit that follows to generalize
these, but this sets the stage.
The recently-introduced pipeline macro takes care of most of the job of a
declarative pipeline, but it's still leaky, since it requires that the
_caller_ create error sum types. This not only exposes implementation
details and so undermines the goal of making pipelines easy to declare and
compose, but it's also one of the last major components of boilerplate for
the lowering pipeline.
My previous attempts at generating error sum types automatically for
pipelines ran into a problem because of overlapping `impl`s for the various
`<S as ParseState>::Error` types; this resolves that issue via
newtypes. I had considered other approaches, including explicitly
generating code to `map_err` as part of the lowering pipeline, but in the
end this is the easier way to reason about things that also keeps manual
`Lower` pipelines on the same level of expressiveness as the pipeline macro;
I want to restrict its unique capabilities as much as possible to
elimination of boilerplate and nothing more.
DEV-13162
I had never intended to avoid pinning nightly. This is an unfortunate thing
to have to do---require a _specific_ version of a compiler to build your
software; it's madness. But the unstable features utilized by TAMER (as
rationalized in `src/lib.rs`) are still worth the effort.
It's not _actually_ that case that we need a specific version of the
compiler, granted; this is outlined in `rust-toolchain.toml`'s
rationale. You should look there for more information; my approach still
utilizes explicit channels via cargo. Unfortunately, I had hard-coded it
previously, putting me in a bit of a bind an unable to override the behavior
without modifying the software.
The reason for this change is that `adt_const_params` has a BC break
involving the introduction of `ConstParamTy`. This is only the second time
I've been bitten by a nightly BC break; the other was the renaming of
`int_log`'s API, as mentioned in
709291b107. This pinning will in fact
mitigate those future issues---TAMER will be able to resolve the issue at
its leisure, and will further be able to continue to build earlier commits
in the future by simply re-bootstrapping with the committed nightly
version.
If you're curious of my rationale for wanting to inhibit toolchain
downloading during build, or use system libraries, have a look at GNU Guix's
approach to building software safely and reproducibly. In particular,
dependencies are also built from source (rather than downloading binaries
from external sources), and builds take place in network-isolated
containers. The `TAMER_RUST_TOOLCHAIN` configure parameter is meant to
facilitate these situations by giving more flexibility to packagers.
DEV-14476
The code utilizing this is flagged, and so the build would output warnings
saying that it was not used. This resolves that (I've been aware of it for
far too long; I'm developing behind the `wip-asg-derived-xmli` flag where I
don't usually see it).
DEV-13162
This generates some documentation helping to describe the lowering pipeline,
since the function type signature can be daunting to those unfamiliar with
it (and I'm sure to the future me too).
DEV-13162
Like the previous commit's removal of the error type, this eliminates the
explicit source token type since we're able to infer it from the pipeline
definition.
DEV-13162
It does not matter what the error of the source is as long as the caller is
able to deal with it, especially given that the particular error is a
property of the source, which is under control of the caller.
DEV-13162
The macro is off-putting and more complicated than the pipeline definitions
themselves (of course), so this tucks it away so that readers are able to
more easily observe the definitions that they're probably looking for
without feeling compelled to try to understand the macro definition.
DEV-13162
All lowering pipelines are now using `lower_pipeline!`. Finally.
The macro does require some refactoring and documentation, but it's working,
and we now have three pipelines whose definitions are smaller than a single
one was previously. I've been hoping to do this for many months, so it's
nice to finally see this come to fruition.
I had been putting it off, but doing so has made it difficult to compose
other parts of the system, not knowing what abstractions I'll have at my
disposal.
DEV-13162
This makes the sink similar to other pipelines without creating a new
ParseState, and so will allow for integrating into the `lower_pipeline!`
abstraction.
DEV-13162
This has been the ultimate goal for the pipeline for some time---the ability
to declaratively define the lowering pipeline in a way that is clear,
concise, and is correct by definition.
The reason that the lowering pipeline required so much boilerplate was
because of the robust types involved, which ensures that everything in the
pipeline is compatible with one-another---it's not possible to construct a
pipeline that will not work.
Of course, there is nuance involved in some cases---I didn't want to include
the `until` clause, which makes it fail the "obviously correct" criterion,
but that can be improved over time.
This only abstracts away `load_xmlo` and `parse_package_xml`; next I'll have
to evolve the abstraction to support lifetimes for `lower_xmli`'s
`AsgTreeToXirf`. That pipeline also ends with a custom sink that really
ought to become its own parser, but I don't want to jump down that rabbit
hole right now, so we may just support custom sinks for now with the intent
of removing it in the future.
This has been a long time coming. The ultimate goal is that you should be
able to look at the parser pipelines to have a clear, high-level overview of
how everything fits together. I'm not generating documentation yet, but
that'll help serve as a guide as well.
DEV-13162
The report acts as the sink for `load_xmlo` and `parse_package_xml`. At the
moment, the type is `()`, and so there's nothing to report on but the
error. But the idea is to add logging via `AirAggregate::Object`, which is
currently just `()`.
This change therefore is only a refactoring---it changes no functionality
but sets up for future changes.
This also introduces consistency with `lower_xmli` in use of `terminal` for
the final operation.
DEV-13162
Diagnostic events need not be errors. While that was the original intent,
it'd also be nice to be able to use the diagnostic system for any type of
logging, where the verbosity level would determine the type of report that
is output (whether source information should be provided).
Then we could have e.g. AirAggregate produce events describing what actions
are occurring, which could be much more useful than a trace in many
contexts, and would be able to operate via a runtime toggle/filter without
having an adverse effect on performance (since the diagnostic rendering
itself is the hit; the underlying data are cheap).
Anyway---I'm addressing this now to generalize the reporter in the lowering
pipeline, so that it can report on not just errors but anything.
DEV-13162
This formats the pipeline to mirror the style of
`parse_package_xml`. Based on the previous commits, the end goal (though
not necessarily now) will be to derive a concise abstraction for all the
lowering pipelines, which means first factoring them into a common form.
DEV-13162
This makes the API of `load_xmlo` much closer to `parse_package_xml`, both
accepting a reporter and distinguishing between recoverable and
unrecoverable errors.
The linker still does not use a reporter and still fails on the first
error, as before; I wanted to keep this change small.
DEV-13162
This allows us to drop `AirIdent::IdentRef`, which in turn allows dropping
`AirIdent` entirely from `AirPkgAggregate`.
This is also a more appropriate abstraction; having to track all the ways in
which `IdentRef` was used can be confusing. This means that `AirIdent` is
true to its name---used only for identifiers. The new token type makes it
very clear where package imports are recognized, and it's also easier to
search for.
DEV-13162
This is the same idea as the previous two commits: get all the lowering
pipelines into the same place so that we can observe commonalities and
attempt to derive an appropriate abstraction.
`lower_xmli` could have invoked `tree_reconstruction` itself, since it has
all the information that it needs to do so, but the idea is that these will
accept sources from the caller. This also demonstrates that sinks need to
be flexible. In an ideal abstraction, perhaps this would be able to produce
an iterator that accepts the first token type and yields the last, which can
then be directed to a sink, but that's not compatible with how the lowering
operations currently work, which requires a single value to be
returned. But if it did work that way, then they'd be able to compose just
as any other parser.
Maybe for the future.
DEV-13162
The previous commit extracted xmlo loading, because that will be a common
operation between `tamec` and `tameld`. This extracts parsing, which will
only used by `tamec` for now, though components of the pipeline are similar
to xmlo loading.
Not only does it need to be removed from `tamec` and better abstracted, but
the intent now is to get all of these things into one place so that the
patterns are obviated and a better abstraction can be created to remove all
of this boilerplate and type complexity.
Furthermore, xmlo loading needs to use reporting and recovery, so having
`parse_package_xml` here will help show how to make that happen easily. I'm
pleased that it ended up being trivial to extract error reporting from the
lowering pipeline as a simple (mutable) callback. I'm not pleased about
the side-effects, but, this works well for now given how the system works
today.
DEV-13162
I want to clean this up a bit further. The motivation is that we need this
for imports in `tamec`.
Eventually this will be cleaned up to the point where it's declarative and
easy to understand---there's a mess of types involved now and, when
something goes wrong, it can be brutally confusing.
DEV-13162
This extracts and decouples the boundary rules from the stack frames
themselves, which not only clarifies what the rules are (and makes them
match the scope diagrams), but paves the way for future isolation.
DEV-13162
This was used for metavariable declaration before scoping was sorted
out. That was just resolved, and so this is no longer needed (and is indeed
not desirable, since it side-steps the scope index and so will not be found
except by `lookup_local_linear`).
DEV-13162
The ASG had its output reduced previously but I had apparently stashed it; I
found it while trying to clean up after so many failed or partial attempts
and the various scoping changes.
The most fundamental issue is that there's too much information: it's very
difficult to interrogate so I seldom look at it, and it slows down Parser
trace output to the point where it's useless on even one of our smallest
systems, generating 1.5GiB of output for a graph of ~10k
objects (via tameld).
DEV-13162
The scope system works with the AIR stack frames, expecting all parent
environments to be on that stack. Since metavariables were (awkwardly) part
of the template parser, that didn't happen.
This change extracts metavariable parsing (with some remaining TODOs) into
its own parser, so that `AirTplAggregate` will be on the stack; then it's a
simple matter of using the existing `AirAggregateCtx` methods to define a
variable and index its shadow scope, which addresses TODOs in the existing
scope test cases.
This also involved separating the tokens from `AirTpl` into `AirMeta`; they
need to be renamed, which will happen in a following commit, since this is
large enough as it is.
Another change that had to be included here, which I wish I could have just
done separately if it wasn't too much work, was to permit overlapping
identifier shadows. Local variables have to cast a shadow so that we can
figure out if they would in turn shadow an identifier (which would be an
error), but they don't conflict with one-another if they don't have a
shared (visible) scope.
`AirAggregate` can be simplified even further, e.g. to eliminate the
expression stack and just use the ctx stack (which didn't previously exist),
but I need to continue; I'll return to it.
DEV-13162
That was being done automatically before this change, but the change that
I'm about to introduce for metavariables will require this distinction, at
the very least to emphasize the behavior of the indexing.
See the next commit for more information.
(The next commit has a bit too much going on, so I wanted to at least
attempt to separate things where it wasn't much work to do so.)
DEV-13162
This finally removes the awkward index from the ASG. This will need much
more documentation and a better organized abstraction, but in the meantime,
previous commit dive into some of the rationale.
In essence: it only really makes sense to have indexing on the ASG itself if
it is used to cache queries or other expensive operations. But that is not
what we were using it for---it was used for caching _lexical_ properties,
which are useful only during parsing for the sake of forming relationships
on the graph. Once those relationships have formed, different types of
indexes will be useful in different lowering, optimization, or querying
contexts.
This formalizes that, and in doing so, ensures that the index is will always
be accurate relative to the content of the ASG. Once the index becomes
separated from it---through the `AirAggregateCtx::finish` operation---then
it is discarded and the ASG exposed.
This is also important because the index is incomplete---it contains only
the information necessary for the parser to carry out its task.
This change was a long time coming, and has reduced ASG to its essence.
DEV-13162
A new AirAggregate parser is utilized for each package import. This
prevents us from moving the index from `Asg` onto `AirAggregateCtx` because
the index would be dropped between each import.
This allows re-using that context and solves for problems that result from
attempting to do so, as explained in the new
`resume_previous_parsing_context` test case.
But, it's now clear that there's a missing abstraction, and that reasoning
about this problem at the topmost level of the compiler/linker in terms of
internal parsing details like "context" is not appropriate. What we're
doing is suspending parsing and resuming it later on for another package,
aggregating into the same destination (ASG + index). An abstraction ought
to be formed in terms of that.
DEV-13162
This was the remaining of my stashed changes that I had mentioned in a
previous commit, but is accomplished differently than I had prototyped. My
initial approach was a bit too klugey: to accept as an argument in various
scope contexts the active parser, as if it were the top stack frame. This
was prototyped before the `AirPkgAggregate` parser was even created.
So we've since created a Pkg parser and now an opaque parser for opaque
idents. There may be other opaque objects in the future.
Because of this change, the parent `AirPkgAggregate` gets stored on the
stack and just naturally becomes part of the lexical scope determination,
and so everything Just Works!
This commit was _supposed_ to be moving the index from `Asg` onto
`AirAggregateCtx`, but I wasn't able to do that because that context is
re-created for each package import currently.
DEV-13162
As evidenced by this change, the tuple syntax was no longer serving us
well. But the real reason for this change is to prepare for the addition of
a fourth field: the index, taken from `Asg`.
DEV-13162
This change means that `asg::air` is now the only module that directly
invokes index-related methods on `Asg`. This clears the way, finally, to
removing the index from `Asg` entirely.
Not only does this result in a less awkward architecture, it also ensures
that lookups are forced to go through the system that understands and
controls lexical scoping, which will be able to give the correct answer.
Of course, the caveat is that the "correct" answer depends on what's
currently on the stack, depending on what type of lookup is being performed,
but those details are still encapsulated within the `asg::air` module and
its tests.
DEV-13162
This is the culmination of a great deal of work over the past few
weeks. Indeed, this change has been prototyped a number of different ways
and has lived in a stash of mine, in one form or another, for a few weeks.
This is not done just yet---I have to finish moving the index out of Asg,
and then clean up a little bit more---but this is a significant
simplification of the system. It was very difficult to reason about prior
approaches, and this finally moves toward doing something that I wasn't sure
if I'd be able to do successfully: formalize scope using AirAggregate's
stack and encapsulate indexing as something that is _supplemental_ to the
graph, rather than an integral component of it.
This _does not yet_ index the AirIdent operation on the package itself
because the active state is not part of the stack; that is one of the
remaining changes I still have stashed. It will be needed shortly for
package imports.
This rationale will have to appear in docs, which I intend to write soon,
but: this means that `Asg` contains _resolved_ data and itself has no
concept of scope. The state of the ASG immediately after parsing _can_ be
used to derive what the scope _must_ be (and indeed that's what
`asg::air::test::scope::derive_scopes_from_asg` does), but once we start
performing optimizations, that will no longer be true in all cases.
This means that lexical scope is a property of parsing, which, well, seems
kind of obvious from its name. But the awkwardness was that, if we consider
scope to be purely a parse-time thing---used only to construct the
relationships on the graph and then be discarded---then how do we query for
information on the graph? We'd have to walk the graph in search of an
identifier, which is slow.
But when do we need to do such a thing? For tests, it doesn't matter if
it's a little bit slow, and the graphs aren't all that large. And for
operations like template expansion and optimizations, if they need access to
a particular index, then we'll be sure to generate or provide the
appropriate one. If we need a central database of identifiers for tooling
in the future, we'll create one then. No general-purpose identifier lookup
_is_ actually needed.
And with that, `Asg::lookup_or_missing` is removed. It has been around
since the beginning of the ASG, when the linker was just a prototype, so
it's the end of TAMER's early era as I was trying to discover exactly what I
wanted the ASG to represent.
DEV-13162
This is in the same spirit as previous commits modifying (or removing)
tests and benchmarks related to accessing the ASG and its indexes directly.
With this change, only `asg::air` uses the indexing and lookup methods on
`Asg`. This will allow me to extract the index from `Asg` entirely and have
`Air` solely responsible for lookup; the graph will be responsible only for,
well, being a graph. Indexing is an optimization strategy.
More information in the commit to follow. But notice how this moving
environment-related concerns away from `Asg` and into AIR, and how the
remaining environment concerns are index-related.
But there is one remaining barrier: to fully move the indexing away from
`Asg`, we have to use an alternative (and complete)
abstraction---AirAggregateCtx with its ability to resolve and introduce
scope based on the stack. The `AirIdent` token subset doesn't yet do that,
and all the work up to this point was in prepartion for doing that. Since
introducing indexing at Root a few commits ago, it's now possible to
proceed.
DEV-13162
These benchmarks were useful as TAMER was in its infancy and I was trying to
gain an intuition for working with Rust. But they are now out of date, and
there are better ways to measure TAMER's performance, including running it
on real-world data (which wasn't possible previously) and through profiling
tools like Valgrind.
With that said, these types of benchmarks _would_ be useful for helping to
dig down into improvements that could be made, at a glance. The problem is,
they aren't testing anything new, and they're also testing something I'm
about to extract from `Asg`. It is not worth the ongoing maintenance cost.
So benchmarks may be reintroduced in the future if they are found to be
valuable.
DEV-13162
The previous commit introduced a duplicate `asg_from_toks`; this just makes
it available publicly for any tests that might utilize AIR to lower the
barrier to writing such tests and provide some guidance in doing so.
DEV-13162
This uses AIR---the ASG's proper public interface now---to construct the
graph for tests, just as all the other modern tests do. This is change
works towards encapsulating index operations (both creation and lookups) so
that the index can be moved off of Asg and into AIR, where it belongs. More
information on that and rationale to come.
DEV-13162
This, finally, introduces identifier pooling in the global environment,
represented by `Root`. All package-level identifiers will be scoped as
such, which at the moment means anything that's not within a template.
As mentioned in recent commits, this does require additional cleanup to
finalize, and some more test will make additional rationale more clear.
It's also worth noting the intent of storing the `ObjectIndex<Root>`---not
only does it mean that the active root can be derived solely from the
current parsing state, but it also means that in the future we can
contribute to any, potentially multiple, roots. I had previously used Neo4J
to effectively diff two dependency graphs between versions in the current
XSLT-based TAMER; I'd like to be able to do that with TAMER in the future,
which is an important concept when considering automated data migration, as
well as querying for the effects of changes.
More to come. I'm hoping this is finally nearing a conclusion and I can
finally tie everything together with package imports. `AirIdent` will be
introduced into the mix soon now too, now that this commit is able to root
them.
DEV-13162
Okay, this is finally distilling into something fairly simple and
reasonable, but I'm not quite there yet.
In particular, the responsibility is simply between `Asg` (as the owner of
the index) and `AirAggregateCtx` (as the owner of the stack frames from
which environments and scope are derived). This was inevitable and I was
waiting for it, but now I have a good idea of how to clean it up and
proceed.
This also doesn't index in root yet (`active_rooting_oi` is still `None` for
`Root`), and I think I may remove `Pool` and just make it `Visible` at that
point, since it won't be going any further anyway. I don't think the
distinction is meaningful and will just complicate implementations.
The tests also need some more cleanup---the assertions ideally would live in
independent tests, and the assertion failure is in a function call rather
than the test (function) itself, so requires a Rust backtrace to locate the
line number of (unless you look at the failure data).
So I suppose this is more of a mental synchronization point than
anything. Nothing's broken, though.
DEV-13162
There's a lot of documentation on this in the commit itself, but this stems
from
a) frustration with trying to understand how the system needs to operate
with all of the objects involved; and
b) recognizing that if I'm having difficulty, then others reading the
system later on (including myself) and possibly looking to improve upon
it are going to have a whole lot of trouble.
Identifier scope is something I've been mulling over for years, and more
formally for the past couple of months. This finally begins to formalize
that, out of frustration with package imports. But it will be a weight
lifted off of me as well, with issues of scope always looming.
This demonstrates a declarative means of testing for scope by scanning the
entire graph in tests to determine where an identifier has been
scoped. Since no such scoping has been implemented yet, the tests
demonstrate how they will look, but otherwise just test for current
behavior. There is more existing behavior to check, and further there will
be _references_ to check, as they'll also leave a trail of scope indexing
behind as part of the resolution process.
See the documentation introduced by this commit for more information on
that part of this commit.
Introducing the graph scanning, with the ASG's static assurances, required
more lowering of dynamic types into the static types required by the
API. This was itself a confusing challenge that, while not all that bad in
retrospect, was something that I initially had some trouble with. The
documentation includes clarifying remarks that hopefully make it all
understandable.
DEV-13162
This begins demonstrating that the root will be utilized for identifier
lookup and indexing, as it was originally for TAME and is currently for the
linker.
This was _not_ the original plan---the plan was to have identifiers indexed
only at the package level, at least until we need a global lookup for
something else---but that plan was upended by how externs are currently
handled. So, for now, we need a global scope.
(Externs are resolved by the linker in such a way that _any_ package that
happens to be imported transitively may resolve the import. This is a
global environment, which I had hoped to get rid of, and which will need to
eventually go away (possibly along with externs) to support loading multiple
programs into the graph simultaneously for cross-program analysis.)
This commit renames the base state for `AirAggregate` to emphasize the fact,
especially when observing it in the `AirStack`, and changes
`AirAggregateCtx::lookup_lexical_or_missing` to resolve from the _bottom_ of
the stack upward, rather than reverse, to prove that the system still
operates correctly with this change in place.
The reason for this direction change is to simplify lookup in the most
general case of non-local identifiers, which are almost all of them in
practice---they'll be immediately resolved at the root once they're
indexed. This can be done because I determined that I will _not_ support
shadowing; rationale for that will come later, but TAME is intended to be a
language suitable for non-programmer audiences as well. Note that
identifiers will be resolved lexically within templates in TAMER, unlike
TAME, which means that the expansion context will _not_ be considered when
checking for shadowing, so templates will still be able to compose without a
problem so long as they do not shadow in their definition context. (I'll
have to consider how that affects template-generating templates later on,
but that's an ambiguous construction in TAME today anyway.)
This _does not_ yet index anything at the root where it wasn't already being
indexed explicitly.
DEV-13162
This requires the name as part of the package definition, which in turn
removes a state (and all the combinations resulting from it) from
AirAggregate, which results in significant complexity reduction for a very
complex part of the system.
Pushing this complexity outward results in a reduction of overall
complexity, and obviates the question of where NIR will receive a generated
name.
DEV-13162
The comment speaks for itself.
My concern is that this will be especially off-putting to people looking at
TAMER and wondering how one could possibly work with this system.
DEV-13162
This is something I've wanted to do for some time, but the system is
becoming hard enough to reason about (with some attempted future changes)
that I require the consistency afforded by this change.
It's not entirely done---as noted by the TODO for `UnnamedPkg`---but it's
close, and then `AirAggregate` will just be a delegating superstate, like
`ele_parse!`.
Importantly, this also puts a package parser on the stack, which will work
better with the stack-based scoping system being developed. It will also
make it easier to fall back to a base case that I had really wanted to
avoid, and will have more information on in the future: root indexing for a
shared global environment for package-level identifiers. (Imports are still
package-scoped, but only in appearance, by contributing to the global
environment of the compilation unit during import. Well, it doesn't do that
yet. The XSLT compiler works in that way.)
DEV-13162
This is one of many changes that have been lingering that I need to start to
break apart in an attempt to commit the confusing and disappointing
conclusion to this package loading madness.
More information to come.
DEV-13162
I had apparently forgotten about this, because I didn't benefit from the
exhaustiveness check; this needs to be eliminated so that this doesn't
happen again, and to provide a proper non-panicking error.
DEV-13162
This reverts commit da7fe96254e425bc7b75f8cf454465b71e27e372.
I'm a fool---this would be pursuant to a future plan that removes AirIdent
opaque tokens. But for now, I need it on IdentDecl and others, which
currently has a `Source` (that I want to go away, as just mentioned), which
contains the same information.
So maybe more to come on this...
DEV-13162
This allows for a canonical package name to be optionally provided to
explicitly resolve a reference against, avoiding a lexical lookup.
This change doesn't actually utilize this new value yet; it just
retains BC. The new argument will be used for the linker, since it already
knows the package that defined an identifier while reading the object file's
symbol table. It will also be used by tamec for the same purposes while
processing package imports.
DEV-13162
-- squashed with --
tamer: asg::air::ir::RefIdent: CanonicalName=SPair
The use of CanonicalName created an asymmetry between RefIdent and
BindIdent. The hope was to move CanonicalName instantiation outside of AIR
and into NIR, but doing so would be confusing and awkward without doing
something with BindIdent.
I don't have the time to deal with that for now, so let's observe how the
system continues to evolve and see whether hoisting it out makes sense in the
end. For now, this works just fine and I need to move on with the actual
goal of finishing package imports so that I can expand templates.
DEV-13162
NOTE: This fixes the aforementioned commit that caused the linker to
temporarily fail (670c5d3a5d at time of
writing). This does introduce an extra forward slash into
`l:dep/preproc:sym/@src`, but that does not appear to cause any
problems. That will eventually go away, so I'm not going to bother with it
any further.
As the `xmlo` file is lowered into AIR, the name will be prefixed with a
leading slash (if necessary, which it is atm) and will emit an
`Air::BindIdent`.
This means that packages will be properly indexed by their canonical name on
load, which will be important when we share this with tamec.
DEV-13162
This change requires every package to have a canonical name, and performs
namespec canonicalization on imports.
Since all package names are canonicalized, this opens the door to being able
to index package names at import, allowing the object to be shared on the
graph and properly reference a package after it has been resolved.
Note that the system tests' canonicalization is relative to the hard-coded
`/TODO` presently; that will change in the near future once `tamec`
generates names from the provided path.
DEV-13162
This introduces, but does not yet integrate, `CanonicalName`, which not only
represents canonicalized package names, but handles namespec resolution.
The term "namespec" is motivated by Git's use of *spec (e.g. refspec)
referring to various ways of specifying a particular object. Names look
like paths, and are derived from them, but they _are not paths_. Their
resolution is a purely lexical operation, and they include a number of
restrictions to simplify their clarity and handling. I expect them to
evolve more in the future, and I've had ideas to do so for quite some time.
In particular, resolving packages in this way and then loading the from the
filesystem relative to the project root will ensure that
traversing (conceptually) to a parent directory will not operate
unintuitively with symlinks. The path will always resolve unambigiously.
(With that said, if the symlink is to a shared directory with different
directory structures, that doesn't solve the compilation problem---we'll
have to move object files into a project-specific build directory to handle
that.)
Span Slicing
------------
Okay, it's worth commenting on the horridity of the path name slicing that
goes on here. Care has been taken to ensure that spans will be able to be
properly sliced in all relevant contexts, and there are plenty of words
devoted to that in the documentation committed here.
But there is a more fundamental problem here that I regret not having solved
earlier, because I don't have the time for it right now: while we do have
SPair, it makes no guarantees that the span associated with the corresponding
SymbolId is actually the span that matches the original source lexeme. In
fact, it's often not.
This is a problem when we want to slice up a symbol in an SPair and produce
a sensible span. If it _is_ a source lexeme with its original span, that's
no problem. But if it's _not_, then the two are not in sync, and slicing up
the span won't produce something that actually makes sense to the user. Or,
worse (or maybe it's not worse?), it may cause a panic if the slicing is out
of bounds.
The solution in the future might be to store explicitly the state of an
SPair, or call it Lexeme, or something, so that we know the conditions under
which slicing is safe. If I ever have time for that in this project.
But the result of the lack of a proper abstraction really shows here: this
is some of the most confusing code in TAMER, and it's really not doing
anything all that complicated. It is disproportionately confusing.
DEV-13162
NOTE: This temporarily breaks `tameld`. It is fixed in a future commit when
names are bound. This was an oversight when breaking apart changes into
separate commits, because the linker does not yet have system tests like
tamec does.
This is preparing for a full transition to requiring a canonical package
name. The previous `Unnamed` variant has been removed and `AirAggregate`
will provide a default `WS_EMPTY` name, as `Pkg` had done before.
The intent of this change is to allow for consulting the index before a
new `Pkg` object is created on the graph, but we're not quite ready for that
yet.
Well, that's not entirely true---the linker can be ready for that. But the
compiler needs to canonicalize import paths relative to the active package
canonical name, which it can't even do yet because tamec isn't generating a
name.
So maybe the linker will be first; it's useful to have that in a separate
commit anyway to emphasize the change.
DEV-13162
...this has apparently been consuming errors for some time. This would
cause the parser to enter an invalid state in some cases and terminate.
This would _not_ permit an invalid link, as the graph would not be correct,
but it was masking the actual error.
This part of linker is in dire need of tests. This also ought to be
replaced with tamec's approach of reporting all errors.
DEV-13162
The previous commit introduced canonical names, and this uses them to index.
The next step will be to utilize those names to look up packages on
definition rather than creating a new package node, so that references to
yet-to-be-defined (or yet-to-be-imported) packages can be resolved on the
graph.
DEV-13162
This is already a concept in the XSLT-based compiler, where each package has
a `package/@name` generated from its path. The same will happen with tamec.
Before we can load packages into the graph, we need canonical identifiers so
that they can be indexed. The next commit will handle indexing using this
information.
DEV-13162
The documentation explains the intent here---existing LaTeX documentation.
The intent was to simply copy the documentation into a LaTeX document based
on the lvspec package that I had created long ago. Of course, that's not
appropriate---we're a DSL and should provide first-class support for
documentation that will compile properly into the target format, whether it
be LaTeX, HTML, JS, or anything else.
DEV-13162
These have been a pain in the ass since TAMER began.
It seemed like a good idea at the time to have static code generated in this
way, but the lack of explicit dependencies just makes this a mess and works
against the operating theory of the system.
Furthermore, the _same_ static fragments were generated for each and every
map package.
There is still a post-link step (standalones) handled in XSLT; the
previously-static code has been moved there. This will eventually be
integrated into tameld itself, once TAMER has facilities for JS generation.
(This was discovered while trying to parent identifiers to packages.)
DEV-13162
With the previous commit using a visitor implemented within the `asg`
module, we can now finally encapsulate the graph. This is a wonderfully
liberating, long-awaited change, since I have been fighting with the lack of
encapsulation for some time; it has made certain changes challenging and has
made the system more difficult to reason about. It also made it impossible
to assert that invariants were _actually_ properly enforced, if things could
just peer into and modify the graph directly, out from underneath the API
that provides those assurances.
This also removes our dependency on Petgraph outside of the `asg`
module. There are no plans to migrate away from it currently; we'll see how
the graph continues to evolve over time and what redundancies are introduced
with our data structures. It may render petgraph unnecessary.
Interestingly, because my DFS implementation is so similar to Petgraph's,
the emitted ordering is _identical_ between this commit and the previous.
DEV-13162
This integrates the new topological sort, replacing the previous
implementation in the linker.
This will now allow encapsulating the graph, finally, and ensures that
future changes can be fully maintained within the `asg` module.
More cleanup will come over time.
DEV-13162
This commit includes plenty of documentation, so you should look there.
It's desirable to describe the sorting that TAME performs as a topological
sort, since that's the end result we want. This uses the ontology to
determine what to do to the graph when a cycle is encountered. So
technically we're sorting a graph with cycles, but you can equivalently view
this as first transforming the graph to cut all cycles and then sorting it.
For the sake of trivia, the term "cut" is used for two reasons: (1) it's an
intuitive visualization, and (2) the term "cut" has precedence in logic
programming (e.g. Prolog), where it (`!`) is used to prevent
backtracking. We're also preventing backtracking, via a back edge, which
would produce a cycle.
DEV-13162
This introduces cycle detection, but it does not yet filter ontologically
permitted cycles, which will be needed prior to utilizing this in `tameld`.
There's a considerable amount of documentation here. While the
implementation is fairly simple, there are important algorithmic decisions,
both in the DFS construction and the derivation of the cycle path from data
that already exists.
This also supports recovery (by ignoring cycles), which can then be utilized
to find more cycles and other errors in the system.
DEV-13162
This is an initial implementation that does not yet produce errors on
cycles. Documentation is not yet complete.
The implementation is fairly basic, and similar to Petgraph's DFS.
A terminology note: the DFS will be ontology-aware (or at least aware of
edge metadata) to avoid traversing edges that would introduce cycles in
situations where they are permitted, which effectively performs a
topological sort on an implicitly _filtered_ graph.
This will end up replacing ld::xmle::lower::sort.
DEV-13162
tameld isn't yet adding edges to Idents from their associated Pkg (see
previous commit), but this formalizes how the ontology will interpret such a
relationship. The idea is that Idents are always owned by Pkgs, but they
may be optionally explicitly rooted, which will be used by a particular type
of DFS walk that is about to be written, which can ignore Root->Pkg and
focus instead on cross edges to Idents.
Though it's not lost on me that now that I'll be introducing a DFS for the
linker, the terms "cross" and "tree" edge now become ambiguous; I used to
call them "ontological X edge", but I had fallen out of that habit; perhaps
I need to reintroduce that rigor.
DEV-13162
This modifies the xmlo reader, xmlo->AIR lowering, and AIR->ASG to introduce
a package for identifiers. It does not yet, however, add edges from the
package to the identifier.
Once edges are added, the DFS will change in undesirable ways, which will
require a new implementation. This is desirable to decouple from Petgraph
anyway, and then will be able to restore the prior single-pass sort+cycle
check.
That will also encapsulate visiting behavior within the `asg::graph` module
and, in turn, allow encapsulating `Asg.graph` finally.
DEV-13162
This doesn't go far enough, but it elaborates a bit---the existing was far
too much of a catch-all. It's important to take advantage of exhaustiveness
checks to ensure each transition is properly accounted for.
This parser is going to get more work over time, including right now, so I'm
not going to go too deep into this yet, but it's be useful (as a reader) to
compare it to e.g. asg::air's parsers' explicit enumeration of states and
favoring of explicit errors over dead state transitions.
DEV-13162
This may now index _any_ type of object, in preparation for indexing package
import paths. In practice, this only makes sense (at least currently) for
`Pkg` and `Ident`.
This generalization also applies to `Asg::lookup_or_missing`.
DEV-13162
Historically, the ASG was better described as a "dependency graph",
containing only identifiers (which are simply called "symbols" in the
XSLT-based compiler). Consequently, it was appropriate for the graph to
have operations specific to identifiers. (Indeed, that's the only type of
object the graph supported.)
Much has changed since then. This cleans things up, and makes parenting
identifiers to root an _explicit_ operation. This will make it easier to
move forward with handling of scope, and importing identifiers into
packages, and removing `Source`, and so on.
DEV-13162
I've been torturing myself trying to figure out how I want to generalize
indexing, lookups, and value numbering in a way that is appropriate for this
project (that is, not over-engineered relative to my needs).
Before I can do much of anything, though, I need to stop having indexing
only as a `Root` thing (previously it wasn't even tied to `Root`). This
makes that change for tamec, but temporarily removes scoping concerns until
I can add more specific types of indexing.
Not only does this allow cleaning up some `Ident`-specific stuff from `Asg`,
but the cleanup also helps to show that portions of the system aren't still
using Root-based globals.
The linker (`tameld`) still uses the old `global` methods for now; those
will eventually go away, but this needs to change to unify both tamec and
tameld once we get to imports as part of the compiler.
DEV-13162
This is needed to then support `@desc` for shorthand desugaring; it's
required by the XSLT-based compiler (and will eventually be required by
TAMER too).
DEV-13708
XIRF->Nir produces `Todo` and `TodoAttr` tokens for many different
things. The previous approach was to ignore those things so that I could
begin adding portions of packages to the graph and observe how that goes.
But now that I'm starting to be able to compile certain packages that
utilize only small subsets of TAME features, I need to have confidence that
I'm fully parsing them. This means rejecting tokens that I haven't yet
gotten to.
DEV-13708
This supports arbitrary documentation as sibling text (mixed content, in XML
terms). The motivation behind this change is to permit existing system
tests to succeed when `Todo | TodoAttr` are both rejected, rather than
having to ignore this.
TAME has always had a philosophy of literate documentation, however it was
never fully realized. This just maintains the status quo; the text is
unstructured, and maybe will be parsed in the future.
Unfortunately, this does _not_ include the output in the `xmli` file or the
system tests. The reason has nothing to do with TAMER---`xmllint` does not
format the output when there is mixed content, it seems, and I need to move
on for now; I'll consider my options in the future. But, it's available on
the graph and ready to go.
DEV-13708
This _only_ re-introduces for PackageStmt since that's all I have tests for
at present. More will be re-added later.
They were previously removed when the attribute parsing was upended in
`ele_parse!`.
This does lose the attribute name, compared to before; that'll ideally be
re-added, and I'll explore options for doing so later, since I also want
them in other contexts. But it needs to be done generically (not
XML-related).
This had to be done before blowing up on TODOs, or system tests would fail.
DEV-13708
This is in preparation for throwing errors (with diagnostic information) on
yet-to-be-supported tokens, so that I can confidently compile individual
packages without worrying that something is just being ignored.
This makes obvious that `ele_parse!` had a different design in mind
previously, and it's now resulting in a lot of boilerplate; I'll address
that in the future once I'm certain requirements have been settled on, since
I've spent far too much time on it to waste more.
DEV-13708
This introduces a new `Doc` object that can be owned by `Expr` (only atm)
and contain what it describes as a concise independent clause. This
construction is not enforced, and is only really obvious today via the
Summary Pages.
There's a lot of latent and unrealized potential in TAME's documentation
philosophy that was never realized, so this will certainly evolve over
time. But for now, the primary purpose was to get `@desc` working on things
like classifications so that `xmli` output can compile for certain
packages.
DEV-13708
These are used by virtually every `ObjectKind`; I've been meaning to do this
for a while, but now that I'm about to introduce a new one (`Doc`), let's
just get it out of the way.
DEV-13708
This doesn't do the actual hard work yet of resolving and loading a package,
but it does place it on the graph and re-derive it into the xmli output.
DEV-13708
This introduces `<match on="foo" />` and `<match on="foo" value="bar" />`,
which are both equality predicates. Other types of predicates are not yet
supported.
This change is a bit messy and leaves a bit to be desired. `NirToAir` is
quite messy and needs some cleanup. There's also the issue of introducing
XML-specific errors in NIR so that users know what things like "subject"
mean, but not being able to do so yet because NIR is agnostic to the source
document type; another layer of abstraction is needed.
But, my priority is first to get derivation of a particularly
expensive (generated) package in our internal systems working first.
DEV-13708
The alternative I was floating was a tagged `Ref` (that is, with an enum
within it), but I settled on this for now, in part for a more concise
notation with the mapping in nir::parse.
We'll see how this evolves. For now, it's not important with the only thing
that uses ref in nir::parse, which is template application.
This was introduced for `match`, which is to come shortly.
DEV-13708
Mike Gerwitz