carbon-language / carbon-lang

Carbon Language's main repository: documents, design, implementation, and related tools. (NOTE: Carbon Language is experimental; see README)
http://docs.carbon-lang.dev/
Other
32.24k stars 1.48k forks source link

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

Open gysddn opened 1 month ago

gysddn commented 1 month ago

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)

Two test cases added for the usage in the interfaces.

josh11b commented 1 month ago

@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.

jonmeow commented 1 month ago

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)

josh11b commented 1 month ago

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.

gysddn commented 1 month ago

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.

jonmeow commented 1 month ago

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.