carbon-lang icon indicating copy to clipboard operation
carbon-lang copied to clipboard

Add `require` node(s) to the parse tree

Open gysddn opened this issue 1 year ago • 5 comments

Added new nodes to the parse tree to handle require declarations

Require is a new bracketed parse node with the following structure:

Require -Impls (New node for expr impls expr statements) - AnyExpr - AnyExpr -RequireIntroducer (Bracket node)

Two test cases added for the usage in the interfaces.

gysddn avatar Aug 09 '24 04:08 gysddn

@josh11b I'm trying to figure out how to review this. Looking at the generics design, I see examples of its use, but I'm having trouble finding what the full syntax is. Is the syntax in the design somewhere that I'm missing? Some of the examples include where, does this overlap with the where parsing you've been looking at?

The argument to require is supposed to match the argument to where, which I recently was working on -- see #4075 . The arguments are described in https://github.com/carbon-language/carbon-lang/blob/trunk/docs/design/generics/details.md#where-constraints -- though the description is spread out across that whole section, without a short summary of just the syntax.

One thing to note, though, that in discussions we've decided that the argument to a where clause is not an expression, and so it has its own distinct interpretation of symbols like == that appear in ordinary expressions.

josh11b avatar Aug 09 '24 21:08 josh11b

The argument to require is supposed to match the argument to where, which I recently was working on -- see #4075 . The arguments are described in https://github.com/carbon-language/carbon-lang/blob/trunk/docs/design/generics/details.md#where-constraints -- though the description is spread out across that whole section, without a short summary of just the syntax.

So to be sure I understand correctly, require actually has a single argument, and that argument is the same as an argument to where? i.e., the impls just happens to be in all the examples?

(if that understanding is correct, probably this change should be reverted since josh11b is already working on where syntax, and this change isn't going to provide the right support)

jonmeow avatar Aug 15 '24 15:08 jonmeow

The argument to require is supposed to match the argument to where, which I recently was working on -- see #4075 . The arguments are described in https://github.com/carbon-language/carbon-lang/blob/trunk/docs/design/generics/details.md#where-constraints -- though the description is spread out across that whole section, without a short summary of just the syntax.

So to be sure I understand correctly, require actually has a single argument, and that argument is the same as an argument to where? i.e., the impls just happens to be in all the examples?

(if that understanding is correct, probably this change should be reverted since josh11b is already working on where syntax, and this change isn't going to provide the right support)

https://github.com/carbon-language/carbon-lang/blob/trunk/proposals/p2760.md#proposal

For now, only the impls forms of where clauses are permitted after require.

Eventually my expectation is that we would accept any kind of where clause after require when parsing, and then add restrictions in the check stage; but for now it is totally fine to only accept impls after require in parse.

josh11b avatar Aug 15 '24 16:08 josh11b

So to be sure I understand correctly, require actually has a single argument, and that argument is the same as an argument to where? i.e., the impls just happens to be in all the examples?

Yeah, I didn't really realize this while making these changes. If the syntax will be provided by @josh11b changes, then I can update this PR or just make another one.

gysddn avatar Aug 16 '24 00:08 gysddn

To be sure we're all on the same page:

  1. Josh is fine with the basic syntax support being added.
    • That is, require <expr> impls <expr>;.
  2. Per my comments, edge cases and incorrect syntax must be tested.
    • The parse tree must be balanced even for parse trees that contain error nodes.

jonmeow avatar Aug 16 '24 15:08 jonmeow

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please comment or remove the inactive label.

This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Dec 09 '24 02:12 github-actions[bot]

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active or becomes active again, please reopen it.

This PR was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

github-actions[bot] avatar Dec 24 '24 02:12 github-actions[bot]