Implement x4::with_local<T>[...] and x4::with_local<T, ID>[...]#73
Merged
saki7 merged 2 commits intoboostorg:developfrom Oct 16, 2025
Merged
Implement x4::with_local<T>[...] and x4::with_local<T, ID>[...]#73saki7 merged 2 commits intoboostorg:developfrom
x4::with_local<T>[...] and x4::with_local<T, ID>[...]#73saki7 merged 2 commits intoboostorg:developfrom
Conversation
Collaborator
Author
|
@yaito3014 Could you review this PR? |
yaito3014
reviewed
Oct 11, 2025
yaito3014
reviewed
Oct 11, 2025
yaito3014
reviewed
Oct 11, 2025
Contributor
yaito3014
left a comment
There was a problem hiding this comment.
LGTM overall, please just resolve chores on test code.
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
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.
Part of #1
This PR implements
x4::with_local<T>[...]andx4::with_local<T, ID>[...], similar to that of Spirit.Qi'sqi::locals<...>.Naming
x4::localsThis is not appropriate.
In Qi era, the name
localswas idiomatic solely because it was an extra parameter given to therulenonterminal:When it comes to X4,
x4::rulehave no place for such parameter, because X4'sruleis a more minimalist entity that consists of only the rule's tag type and the attribute type. So it should be implemented as a directive. Therefore, the name "locals" is no longer appropriate and we need to choose the best name.x4::localThis is OK, but if we carefully think about its semantics then it sounds awkward because it is very vague what "local" it does point to. What I'm trying to say here is that the term "local" itself is vague, but when it's used in a directive, it gets increasingly weird:
What "local int" does it refer to? How can
some_parserbe relevant to some "local int"? Is the underlying parser "local int"? Does the underlying parser contain some "local int"? Is that an inner variable or an outer variable?We could insist that it IS a local variable similar to that of Qi, but that's a very arbitrary definition and none of the above questions can be answered clearly.
x4::with_localThis is the preferred naming in this PR.
Context design
Initially I implemented
x4::with_local<ID, T>[...], where theIDparameter is mandatory and must be supplied by the users, similar tox4::with<ID>(var)[p]. However, there are many use cases where only a single local variable is needed for grammar definition.The current design is to default the
IDtype tox4::contexts::local_var, which refers to always the innermost variable created by potentially nested invocation ofwith_localparsers. This PR also provides a convenient accessorx4::_local_var(ctx)that fetches the corresponding variable. In this scenario, simplyx4::with_local<T>[...](withIDparam omitted) can be utilized.For more complicated scenario, users can supply arbitrary context types:
x4::with_local<T, A_ID>[x4::with_local<U, B_ID>[...]]. This way, any number of local variables can be defined and all of them can be fetched simultaneously by specifying corresponding tag likex4::get<A_ID>(ctx)orx4::get<B_ID>(ctx).Multiple local variables in a single directive
Multiple local variables in a single directive is not supported in X4 by design.
The old feature in Qi was able to have multiple local variables specified in a single chunk:
However this required users to access the variable using a surrogate key, i.e.,
qi::_a,qi::_b,qi::_c, and so on. In my experience, this was an unbearable nightmare because it was very unintuitive to correlate the sequential index within the grammar definition, as the real index is buried deep inside the rule's declaration.If we keep that design then we would have this in X4:
I consider this very unpractical.
As a solution we can simply supply some predefined struct with arbitrary number of elements:
This way it's much cleaner, and of course,
std::tuple<...>can also be used for this purpose.