When set, this will cause runner ACK failures to occur approximately every
1/N requests. This allows manually testing conditions that would normally
only occur under severe resource contention.
DEV-10806
When the client requests a reload (e.g. on ACK wait failure), we should not
terminate, since we are expecting to attempt the request again.
This was broken by the previous commit.
DEV-10806
It's a tad embarrassing that this has been eluding me for quite some
time. I happened to run into it while testing the previous commit, which in
turn only existed because I was trying to optimize runner performance.
We'd have situations where, following a runner reload (exit code 129 =
SIGHUP), the build would simply hang indefinitely. Apparently, `tame`, in
`command-runner`, blocks on a `read` without a timeout, expecting that the
FIFO attached to stdin will close if the runner crashes. Maybe that used to
be the case, but that is no longer true today.
Because of that, the FIFO stays open, and read continues to block, waiting
for `DONE`.
Now, `tamed`, when seeing that a runner has crashed (which could have been
due to a reload), will check to see if that runner is marked busy. If so,
that means that the client `tame` did not see `DONE`, because it did not
clear the flag via `command-runner`'s `mark-available.` To notify the
client that someone went wrong, `tamed` will inject a `DONE` into the output
FIFO, which will allow the client to fail properly.
`dslc` catches exceptions and should output `DONE` under normal operating
conditions. However, since some of our systems require so much memory to
build, we may encounter the OOM killer. In that case, the process has no
time to recover (it is killed with SIGKILL), and therefore cannot output
`DONE`. I suspect this is what has been happening to cause occasional build
hangs.
One final thing to clean this up: since we're properly handling reloads now,
based on this commit and the immediately preceding one, we can suppress the
warning when the code is 129 (see comments).
DEV-10806
The `tame` client has the ability to request a runner reload by issuing
SIGHUP to the runner PID. This will cause `tamed` to kill the runner and
respawn it.
There were situations where this process hung or did not operate as
expected; the process was not reliable. This does a number of things to
make sure the process is in a good state before proceeding:
1. The HUP trap is set a single time, rather than being reset each time
the signal is received for the runner.
2. A `reloading` flag is set by the `tame` client when reloading is
requested, and the flag is cleared by `tamed` when spawning a new
runner. This allows the client to wait until the reload is complete,
and bail out otherwise. Without this, there is a race, where the
client may proceed to issue additional requests before the original
runner terminates.
3. Kill the runner with SIGKILL (signal 9). This gives no opportunity for
the runner to ignore the signal, and gives us confidence that the
runner is actually going to be killed.
This may have caused errors that look like this (where 129 is the HUP
reload, which is expected):
warning: failed runner 0 ack; requesting reload
warning: runner 0 exited with code 129; restarting
error: runner 0 still unresponsive; giving up
The last line may also be omitted, with instead an empty `xmlo` being
generated.
DEV-10806
This uses `exec` so that the process is replaced, simplifying process
management (we don't have to worry that the script will die but leave the
child hanging). I'm not sure why I didn't do this originally.
DEV-10806
I have long forgotten about this system. It converts typedefs into a more
generic domain, but the way in which it does so causes duplicate domains,
for two reasons:
- Both `preproc:mkdomain` and the caller (`preproc:expand`) recurse into
unions and generate domains; and
- Each `preproc:expand` pass generates domains.
So, for example, if there are two `preproc:expand` passes on a union, then
the outer typedef (union) will have domains generated twice (once for each
pass), and the inner typedefs will have domains generated four times (for
each expansion pass, and twice for each pass).
This resolves the issue before the next commit makes further changes to move
this into a generated header file.
This was used before __pkguniq to generate identifiers. Back then, I seemed
to think determinism was a problem and that randomness was desirable for
helping to ensure uniqueness between packages. That was a mistake; we
_want_ a deterministic system (which is far easier to debug and verify the
results of), we just want uniqueness.
DEV-14965
This modifies the XSLT-based compiler to generate ids that are expected to
be unique across packages. No such guarantee exists today; `generate-id()`
relies on the position of the node within a tree, which could easily be the
same across multiple compiler invocations for separate packages.
This situation seldom occurs, but has happened with increased frequency
lately in a system with >1000 packages. It is more likely to occur in
packages that are very similar to one-another or where the beginning of the
package is similar (such as packages used as configuration for taxes for
each individual state).
This derives a SHA-256 hash from the canonical package name (well, not
canonical acccording to TAMER, but close: without the leading slash),
truncating it to 32 bits. I used a birthday attack to estimate what the
size of this value ought to be: sqrt(2^32) = 65536, which is way more
packages than the poor XSLT-based compiler is going to handle.
If ever it needs to be increased due to conflicts, that is simple enough.
DEV-14965
This ensures that each metavariable defined within a template (a template
parameter) has, by the time that the template definition is ended, at least
one reference to each metavariable.
This has practical benefits---ensuring that you haven't forgotten to use a
metavariable; ensuring that you clean up when code is removed; and ensuring
that you didn't accidentally delete some reference that you didn't intend to
(at least in the case of _all_ references...)---but the rationale for it in
this commit is a bit different. More on that below.
This does introduce the negative effect of making it more difficult to
consume inputs without utilizing them, acting more like a relevant type
system (in terms of substructural type systems and with respect to
metavariables, at least). You can for now reference them in contexts that
would reasonably have no effect on the program or be optimized away, but in
the future it'd be nice to explicitly state "hey this isn't intended to be
used yet", especially if you're creating shells of templates, or trying to
maintain BC in a particular situation. But more on that in the future.
For now, the real reason for this change is because of how I intend for
template expansion to work: by walking the body. Rather than having to
check both the parameters of the template and then expand the body
separately, we can simply trust that each parameter is referenced. Then,
after rebinding metavariable references, we can perform the same check on
the expansion template to see if there were arguments provided that do not
correspond to parameters.
This also adds flexibility with parameters that are used
conditionally---we'll be able to have conditionally required parameters in
error reporting.
More information on this is coming, though; it'll be included in the docs of
the commit that introduces the changes.
DEV-13163
The number of files that had to be changed here emphasizes the need for a
better abstraction for indirection in the future. I just don't have the
time to do so yet.
This introduces the same type of indirection that was needed for abstract
bindings: we need to be able to distinguish between a metavariable
identifier's lexical value as a _string_ vs. a _reference_; the former is
how it was interpreted previously, which meant that interpolation would not
cause an edge to be created to the metavariable, which in turn means that
the metavariable was not referenced. This in turn blocked a change to
require that all metavariables be referenced.
I'm climbing back up my stash stack.
DEV-13163
This creates a new `AirDocAggregate` child parser and handled documentation
in a consistent way for all object types. The logic that was previously
object-specific now lives on the edge hooks that did not exist when doc
parsing was originally introduced.
This prepares for doc refs so that metavariables can be used for doc
generation (most notably for interpolated documentation strings in
templates).
This also introduces a new error type specifically for meta, giving guidance
to use `<text>`. I don't like that it builds in source language assumptions
in the help message provided, however I'd rather it be able to provide some
guidance to people, especially given that the XML source language is the
only one recognized by TAMER at the moment.
DEV-13163
Only internal modules ought to be able to mutate objects. This is important
now with the use of the `f::Map` trait, since traits don't allow for private
methods, which were previously the means of maintaining encapsulation.
DEV-13163
See included documentation for more information. This completes `MetaState`
inference, for now, until we are able to be notified when missing
identifiers acquire bindings.
DEV-13163
This moves us closer toward template expansion by allowing us to look at an
`Expr` and determine whether it requires any expansion at all. Naturally,
if something doesn't require expansion, we're able to duplicate it in the
tree without duplicating any data on the graph by simply adding a tree edge
to it. But, that's for a future commit; this just tracks the status.
Just as with some recent previous changes, we're not yet notifying `Expr`s
when an `Ident` is no longer `Missing`, and so order matters for now. This
will be rectified in the future.
DEV-13163
This prepares for the addition of more fields.
I don't want to sacrifice exhaustiveness checking, which is unfortunately
more verbose for structs when ignoring fields. This verbosity will be
adopted in certain situations, like shown in this patch, to ensure that
we're made aware of areas of the program that require our attention when
fields are added.
This also shows that I'll need a struct arm for `impl_mono_map!`.
DEV-13163
This removes the token of lookahead and just does what needs to be done in a
more clear manner. There is no room for interpretation in what this is
doing now, and `TplEnd` delegates to `close` just as this does.
DEV-13163
We are now able to have confidence that the graph is properly constructed
with or without a reference span depending on whether the edge is a tree or
cross edge. Consequently, the span is now a reliable indicator of whether
an edge is tree or cross in _all_ cases, not just in dynamic ones.
In fact, this did catch a couple cases where the spans were _not_ properly
applied.
This will in turn give me confidence moving forward with static analysis
based on the graph (and edge hooks).
I could go further than this by introducing a new span type in place of
`Option<Span>`, which would also allow me to get rid of having two methods
on `Asg`, but I want to move on for now; this can be cleaned up more later
on.
It's also worth noting that this explicit method-based distinction between
edge types also means that each caller will carefully consider how the
operation affects the graph. Previously, that consideration was framed very
differently: "do I need a contextual span or not?". That's not the right
question to ask.
DEV-13163
When this span was originally introduced for an edge, I wasn't sure whether
it may have use cases beyond just references. At this point, I'd rather be
explicit; it can evolve over time if need be.
There is nothing yet that prevents a span from being added to a tree edge,
though.
DEV-13163
This also hoists `?` out of the inner match arms to make control flow easier
to follow. I try to balance convenience with brutal explicitness, to make
control flow more obvious and make it easier to reason about what the system
is doing at every point in the program.
DEV-13163
This allows removing the `diagnostic_panic!` from the `Ident` edge handling
for `Tpl`.
I do need to create a generic sum type macro at some point, considering how
extensively these are used.
DEV-13163
Oh, identifiers. What a mess you make of the graph. As indirection tends
to do, I suppose.
This still needs some cleanup; I wanted to get something working first. In
particular:
- Common operations need refactoring;
- Having all this code in the edge definition makes for a mess;
- `Ident` references really ought not be returned by `Ident::definition`
since an identifier will never be a definition.
...I'm sure there's more.
DEV-13163
This renames the previous operation to `definition_narrow` and creates a new
`definition` that does not attempt to narrow. This is important for
distinguishing between a missing defintion and a definition of a given type,
and will be important now that we're beginning to reason about identifiers
that were resolved during parsing.
DEV-13163
I changed my mind from the previous commit---I do in fact want to inhibit
edge additions in some cases, to override what is added (e.g. to wrap a
template application in an Expr workspace).
The approach that I was trying to take was to override `from_oi` and return
it to the caller, but with all the dynamic edges, that ended up being a lot
more code than I feel was worth it; it was too complex. This is a much
simpler (and more flexible) approach---we simply carry out whatever graph
operations we want in `pre_add_edge` using the provided `asg` reference, and
then after we're done, simply omit `commit` on the original.
DEV-13163
This does two things:
1. Removes callback; it didn't add anything of practical value.
The operation will simply be performed as long as no error is provided
by the callee.
2. Consolodates three arguments into `ProposedRel`. This makes blocks in
`object_rel!` less verbose and boilerplate-y.
I'll probably implement `TplShape::Unknown` via the dynamic `Ident` `Tpl`
edge before continuing with any cleanup. This is getting pretty close to
reasonable for future implementations.
DEV-13163
This helps to remove some boilerplate. Testing this out in
`asg::graph::object::tpl` before applying it to other things; really `Map`
can just go away entirely then since it can be implemented in terms of
`TryMap`, but maybe it should stick around for manual impls (implementing
`TryMap` manually is more work).
DEV-13163
At least not how most people expect functors to be. I'm really just using
this as a map with powerful inference properties that make writing code more
pleasent.
And I need fallible methods now too.
DEV-13163
Things are starting to get interesting, and this shows how caching
information about template shape (rather than having to query the graph any
time we want to discover it) makes it easy to compose shapes.
This does not yet handle the unknown case. Before I do that, I'll want to
do some refactoring to address duplication in the `tpl` module.
DEV-13163
This enforces the new constraint that templates expanding into an `Expr`
context must only inline a single `Expr`.
Perhaps in the future we'll support explicit splicing, like `,@` in
Lisp. But this new restriction is intended for two purposes:
- To make templates more predictable (if you have a list of expressions
inlined then they will act differently depending on the type of
expression that they are inlined into, which means that more defensive
programming would otherwise be required); and
- To make expansion easier, since we're going to have to set aside an
expansion workspace ahead of time to ensure ordering (Petgraph can't
replace edges in-place). If we support multi-expansion, we'd have to
handle associativity in all expression contexts.
This'll become more clear in future commits.
It's nice to see all this hard work coming together now, though; it's easy
now to perform static analysis on the system, and any part of the graph
construction can throw errors with rich diagnostic information and still
recover properly. And, importantly, the system enforces its own state, and
the compiler helps us with that (the previous commits).
DEV-13163
This formalizes the previous commit a bit more and adds documentation
explaining why it exists and how it works. Look there for more
information.
This has been a lot of setup work. Hopefully things are now easier in the
future. And now we have nice declarative type-level hooks into the graph!
DEV-13163
This change is the first to utilize matching on edges to determine the state
of the template (to begin to derive its shape).
But this is notable for my finally caving on `min_specialization`.
The commit contains a bunch of rationale for why I introduced it. I've been
sitting on trying it for _years_. I had hoped for further progress in
determining a stabalization path, but that doesn't seem to be happening.
The reason I caved is because _not_ using it is a significant barrier to
utilizing robust types in various scenarios. I've been having to work
around that with significant efforts to write boilerplate code to match on
types and branch to various static paths accordingly. It makes it really
expensive to make certain types of changes, and it make the code really
difficult to understand once you start to peel back abstractions that try to
hide it.
I'll see how this goes and, if it goes well, begin to replace old methods
with specialization.
See the next commit for some cleanup. I purposefully left this a bit of a
mess (at the bottom of `asg::graph::object::tpl`) to emphasize what I'm
doing and why I introduced it.
DEV-13163
This allows for a declarative matching on edge targets using the trait
system, rather than having to convert the type to a runtime value to match
on (which doesn't make a whole lot of sense).
See a commit to follow shortly (with Tpl) for an example use case.
DEV-13163
Since we're statically invoking a particular ObjectKind's method, we already
know the source type. Let's pre-narrow it for their (my) convenience.
DEV-13163
There's a lot to say about this; it's been a bit of a struggle figuring out
what I wanted to do here.
First: this allows objects to use `AsgObjectMut` to control whether an edge
is permitted to be added, or to cache information about an edge that is
about to be added. But no object does that yet; it just uses the default
trait implementation, and so this _does not change any current
behavior_. It also is approximately equivalent cycle-count-wise, according
to Valgrind (within ~100 cycles out of hundreds of millions on large package
tests).
Adding edges to the graph is still infallible _after having received
permission_ from an `ObjectIndexRelTo`, but the object is free to reject the
edge with an `AsgError`.
As an example of where this will be useful: the template system needs to
keep track of what is in the body of a template as it is defined. But the
`TplAirAggregate` parser is sidelined while expressions in the body are
parsed, and edges are added to a dynamic source using
`ObjectIndexRelTo`. Consequently, we cannot rely on a static API to cache
information; we have to be able to react dynamically. This will allow `Tpl`
objects to know any time edges are added and, therefore, determine their
shape as the graph is being built, rather than having to traverse the tree
after encountering a close.
(I _could_ change this, but `ObjectIndexRelTo` removes a significant amount
of complexity for the caller, so I'd rather not.)
I did explore other options. I rejected the first one, then rejected this
one, then rejected the first one again before returning back to this one
after having previously sidelined the entire thing, because of the above
example. The core point is: I need confidence that the graph isn't being
changed in ways that I forgot about, and because of the complexity of the
system and the heavy refactoring that I do, I need the compiler's help;
otherwise I risk introducing subtle bugs as objects get out of sync with the
actual state of the graph.
(I wish the graph supported these things directly, but that's a project well
outside the scope of my TAMER work. So I have to make do, as I have been
all this time, by layering atop of Petgraph.)
(...I'm beginning to ramble.)
(...beginning?)
Anyway: my other rejected idea was to provide attestation via the
`ObjectIndex` APIs to force callers to go through those APIs to add an edge
to the graph; it would use sealed objects that are inaccessible to any
modules other than the objects, and assert that the caller is able to
provide a zero-sized object of that sealed type.
The problem with this is...exactly what was mentioned above:
`ObjectIndexRelTo` is dynamic. We don't always know the source object type
statically, and so we cannot make those static assertions.
I could have tried the same tricks to store attestation at some other time,
but what a confusing mess it would be.
And so here we are.
Most of this work is cleaning up the callers---adding edges is now fallible,
from the `ObjectIndex` API standpoint, and so AIR needed to be set up to
handle those failures. There _aren't_ any failures yet, but again, since
things are dynamic, they could appear at any moment. Furthermore, since
ref/def is commutative (things can be defined and referenced in any order),
there could be surprise errors on edge additions in places that might not
otherwise expect it in the future. We're now ready for that, and I'll be
able to e.g. traverse incoming edges on a `Missing->Transparent` definition
to notify dependents.
This project is going to be the end of me. As interesting as it is.
I can see why Rust just chose to require macro definitions _before_ use. So
much less work.
DEV-13163
AIR is no longer able to explicitly add edges without going through an
object-specific `ObjectIndex` API. `Asg::add_edge` was already private, but
`ObjectIndex::add_edge_{to,from}` was not.
The problem is that I want to augment the graph with other invariants, such
as caches. I'd normally have this built into the graph system itself, but I
don't have the time for the engineering effort to extend or replace
Petgraph, so I'm going to build atop of it.
To have confidence in any sort of caching, I need assurances that the graph
can't change out from underneath an object. This gets _close_ to
accomplishing that, but I'm still uncomfortable:
- We're one `pub` addition away from breaking these invariants; and
- Other `Object` types can still manipulates one-anothers' edges.
So this is a first step that at least proves encapsulation within
`asg::graph`, but ideally we'd have the system enforce, statically, that
`Objects` own their _outgoing_ edges, and no other `Object` is able to
manipulate them. This would ensure that any accidental future changes, or
bugs, will cause compilation failures rather than e.g. allowing caches to
get out of sync with the graph.
DEV-13163
The fixpoint tests for `meta-interp` are finally working. I could have
broken this up more, but I'm exhausted with this process, so, you get what
you get.
NIR will now recognize basic `<text>` and `<param-value>` nodes (note the
caveat for `<text>` in the comment, for now), and I finally include abstract
binding in the lowering pipeline. `xmli` output is also now able to cope
with metavariables with a single lexical association, and continues to
become more of a mess.
DEV-13163
This is a really obvious problem in retrospect, which makes me feel rather
silly.
The output was useful, but I don't have time to deal with this any further
right now. The comments in the commit explain the problem---that the output
ends up being interpolated as part of the fixpoint test, in an incorrect
context, and so the code that we generate is invalid. Also goes to show why
the fixpoint tests are important.
(Yes, they're still disabled for meta-interp, I'm trying to get them
enabled.)
DEV-13163
The provided documentation provides rationale, and the use case is the
ontree change. I was uncomfortable without the exhaustive match, and I was
further annoyed by the lack of easy `ObjectIndex` narrowing.
DEV-13163
This introduces the ability to specify an edge ordering for the ontological
tree traversal. `tree_reconstruction` will now use a
`SourceCompatibleTreeEdgeOrder`, which will traverse the graph in an order
that will result in a properly ordered source reconstruction. This is
needed for template headers, because interpolation causes
metavariables (exposed as template params) to be mixed into the body.
There's a lot of information here, including some TODOs on possible
improvements. I used the unstable `is_sorted` to output how many template
were already sorted, based on one of our very large packages internally that
uses templates extensively, and found that none of the desugared shorthand
template expansions were already ordered. If I tweak that a bit, then
nearly all templates will already be ordered, reducing the work that needs
to be done, leaving only template definitions with interpolation to be
concerned about, which is infrequent relative to everything else.
DEV-13163
Well, this is both good news and bad news.
The good news is that this finally produces the expected output and
reconstructs sources from interpolated values on the ASG. Yay!
...the bad news is that it's wrong. Notice how the fixpoint test is
disabled.
So, my plan was originally to commit it like this first and see if I was
comfortable relaxing the convention that `<param>` nodes had to appear in
the header. That's nice to do, that's cleaner to do, but would the
XSLT-based compiler really care? I had to investigate.
Well, turns out that TAMER does care. Because, well over a decade ago, I
re-used `<param>`, which could represent not only a template param, but also
a global param, or a function param.
So, XML->NIR considers all `<param>` nodes at the head of a template to be
template parameters. But after the first non-header element, we transition
to another state that allows it to be pretty much anything.
And so, I can't relax that restriction.
And because of that, I can't just stream the tree to the xmli generator,
I'll have to queue up nodes and order them.
Oh well, I tried.
DEV-13163
I'm not sure how I overlooked this previously, and I didn't notice until
trying to generate xmli output. I think I distracted myself with the
use of dangling status, which was not appropriate, and that has since
changed so that we have a dedicated concept.
This introduces the term "instantiation", or more specifically "lexical
instantiation". This is more specific and meaningful than simply
"expansion", which is what occurs during instantiation. I'll try to adjust
terminology and make things more consistent as I go.
DEV-13163
This logic ought to live alongside other definition logic...which in turn
needs its own extraction, but that's a separate concern.
This makes the definition of abstract identifiers very similar to
concrete. But, treating these as dangling, even if that's technically true,
has to change---we still want an edge drawn to the abstract identifier via
e.g. a template since we want the graph to mirror the structure of what it
will expand into concretely. I didn't notice this problem until trying to
generate the xmli for it.
So, see the commit to follow.
DEV-13163
This handles the common cases for meta, which includes what interpolation
desugars into. Most of this work was in testing and reasoning about the
issue; `asg::graph::visit:ontree::test` has a good summary of the structure
of the graph that results.
The last remaining steps to make this work end-to-end is for NIR->AIR to
lower `Nir::Ref` into `Air::BindIdent`, and then for `asg::graph::xmli` to
reconstruct concatenation lists. I'll then be able to commit the xmli test
case I've been sitting on, whose errors have been guiding my development.
DEV-13163
Mike Gerwitz