Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support capture existence testing and simple control flow #15

Closed
wants to merge 5 commits into from

Conversation

tausbn
Copy link
Collaborator

@tausbn tausbn commented Nov 8, 2021

Adds support for if statements and a prefix operator ? that can be used to test for the presence of a given capture.

Thus, a stanza like

(module . (expression_statement)? @first) @root
{
  node @root.node
  if ?@first {
    attr (@root.node) first_expr = @first
  }
}

will set the first_expr attribute only in the case where the first statement in the input is in fact an expression.

In addition to this, this PR also adds functions for the usual boolean operators and, or, and not.

Can be reviewed commit-by-commit.

A fairly straightforward implementation. Rather than draining the entire
list of parameters from `exec.function_parameters` when calling a
function, we just drain the last `N` where `N` is the number of
arguments passed to the function.

The added test (hopefully) demonstrates that this works correctly.
Without the changes in this PR, execution would panic when the outer
`replace` call was called without arguments (as these would have been
purged by the second call).
Adds the prefix operator `?` which coerces capture
existence/non-existence into a boolean value. Thus, `? @foo` is `#true`
if the capture `@foo` exists, and is `#false` otherwise.

Because a capture that _doesn't_ exist results in an error if evaluated,
this behaviour cannot be implemented in terms of the existing
function call mechanism.

The syntax itself is also not entirely ideal. For ease of parsing, it's
better for it to be a prefix operator, but `?@foo` is not the prettiest
combination of symbols.

Alternatively, one might dispense with the `@` in this expression, and
write it simply as `?foo`, though that might cause confusion if there
is also a local variable called `foo` in the given stanza.
Adds the functions `and`, `or`, and `not` to the standard library. The
first two of these accept any number of arguments. The third expects a
single argument.
@dcreager
Copy link
Contributor

dcreager commented Nov 9, 2021

As an fyi @hendrikvanantwerpen has also been working on lazy evaluation in the lazy-evaluation branch. I know that involves some conditional logic under the covers to support evaluating scan statements lazily. We should make sure these two proposals / PRs don't conflict with each other.

@tausbn
Copy link
Collaborator Author

tausbn commented Nov 9, 2021

As an fyi @hendrikvanantwerpen has also been working on lazy evaluation in the lazy-evaluation branch. I know that involves some conditional logic under the covers to support evaluating scan statements lazily. We should make sure these two proposals / PRs don't conflict with each other.

Yep, I'm aware (though that branch doesn't seem to be in a good state for reviewing yet, so I haven't looked at its contents in any detail).

The way I see it, this PR is just a proposal. I'm personally not enthused with the ?@foo syntax, and so I wouldn't expect to see this merged in its current form (or at all, for that matter). Mostly I just wanted it to spark some discussion about the benefits of adding control flow. (And just implementing my suggestion seemed like the most straightforward way to do so. 🙂)

@hendrikvanantwerpen
Copy link
Collaborator

Based on this proposal I have implemented #20, which adds conditionals and list iteration, as well as improves handling of captures of patterns with suffixes such as ? and *. I therefore propose to close this PR in favor of #20.

@tausbn
Copy link
Collaborator Author

tausbn commented Nov 24, 2021

Agreed! Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants