refactor: introduce TemplateProgram between AST and CompiledProgram - #388
Open
apoelstra wants to merge 8 commits into
Open
refactor: introduce TemplateProgram between AST and CompiledProgram#388apoelstra wants to merge 8 commits into
TemplateProgram between AST and CompiledProgram#388apoelstra wants to merge 8 commits into
Conversation
Changes the `as_inner` method to return an `&Arc` rather than an `&str`, which makes more sense given the name, and is also useful in a couple of places. Then * Add `PartialEq` between the wrapper types and `str`, eliminating a ton of .as_inner calls entirely. * Add a `from_ident` method which converts an `Identifier` into the type, which is a common operation throughout the codebase to "strongly type" an identifier. * Add a `from_str` method to cover the remaining cases where we really do need an explicit &str
The existing code has an interesting sort of type confusion: we parse Arguments, UnresolvedValues, and WitnessValues by means of a ParserVisitor which calls into chumsky to parse the key. For each of these cases, we expect an identifier, which depending on the map, may reference either a parameter or a witness. In both cases, we put them into the WitnessName type, which covers both but does not distinguish between them. To achieve this, in src/parse.rs we implement ChumskyParse on WitnessName, by parsing an identifier and then interpreting it as a WitnessName. (We never use this codepath when parsing actual programs; in actual programs identifiers are just Identifiers until a later resolution stage; this ChumsyParse impl is only used as a helper for serde-deserialization of .args and .wit files.) (In programs, witnesses and parameters must be prefixed with `witness::` or `param::`, and these have their own parsing path.) *However*, in the following commits, we will update WitnessNames so that it distinguishes between parameters and witnesses, because we are going to delay parameter resolution until after compilation. Once we do this, the ChumskyParse impl for WitnessName will no longer be tenable, because it needs to construct a WitnessName but it doesn't know whether the identifier it's converting is supposed to be a parameter or a witness name. This commit simply refactors code and does not change any data structures or behavior.
Our "template program" is really a "templated SimplicityHL AST". I'd
like to introduce a new "templated Simplicity Program" for which the
name TemplateProgram would be a better fit. So rename this out of the
way.
This commit can be reproduced with
find bitcoind-tests external-jet-lib-example src fuzz tests \
-type f \
\( -name '*.rs' -o -name '*.md' \) \
-exec sed -i s/TemplateProgram/TemplateAst/g {} \;
Over the next couple commits I am going to generalize WitnessName to cover multiple possibilities (both named witnesses and parameters, in a new "template program" type). Start by moving stuff around, and replacing the macro-generated accessors with hand-written ones.
We are already using this type for both parameters and witnesses. Update the name to reflect that. This commit is just a search-and-replace. It is hopefully easy to review even though it's big, for that reason.
There are two purposes to this trait: * Moves a bunch of function definitions out of macro-generated code and into default trait methods; this is easier to read and improves LSP integration * Introduces the ident_to_key method, which will allow Arguments and WitnessValues to both be created from the UnresolvedValues map. Currently this is done using a From<HashMap> bound, but when we extend WitnessNames to hold parameters as well as witness names, this From bound won't be sufficient. Also, change UnresolvedValues to be keyed by Identifier rather than by WitnessName. Until we resolve the name, we don't know whether we have a parameter name or a witness name, so until then, Identifier is the more correct type.
After all our prep work this is fairly easy to do.
apoelstra
force-pushed
the
2026-08/add-template-program
branch
2 times, most recently
from
August 13, 2026 23:45
1d0a572 to
4881f30
Compare
Currently this doesn't do anything. It just adds an extra step to compilation. The next commit will rearrange things such that instantiation happens -after- compilation.
apoelstra
force-pushed
the
2026-08/add-template-program
branch
from
August 14, 2026 02:54
4881f30 to
9942a5a
Compare
stringhandler
approved these changes
Aug 14, 2026
stringhandler
left a comment
stringhandler
left a comment
Contributor
There was a problem hiding this comment.
ACK 9942a5a Ran cargo test locally
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR does a number of refactors ahead of introducing quoting and canonical CMRs. The goal of these changes is to rearrange compilation.
Rather than the process being
Templated AST (
TemplateProgram) -> instantiated AST -> compiled programinstead it will be
Templated AST -> Templated compiled program -> compiled program
I tried to reduce churn but there are a couple renames that come with this:
TemplateProgrambecomesTemplateAst(and a newTemplateProgramtype is introduced)WitnessNames, which is the name of the witness type for our compiled programs, becomesTemplateProgramWitness, and is expanded to track both witness names and parametersIn fact,
WitnessNamesis already (ab)used to store both witness names and parameters, which makes these refactors a little easier. But this appears to be a form a code reuse rather than a deliberate API choice; the currentWitnessNamestruct is just a wrapper aroundArc<str>that does not track what kind of object it's holding, and we distinguish based on what context it appears in. Because we instatiate before compiling, any remainingWitnessNamesin a compiled program are definitely witness names and not parameter names. This PR changes that, so we need to extend the type.Once these refactors are done, the actual rearranging of the instantiation step is suprisingly easy. But it will have to wait for the next PR.
This PR has no behavior changes, does not introduce any new functionality, and does not use any features of the upcoming rust-simplicity.