Skip attributes with no related item.

[gilbens-starkware]

This change is 

[orizi]

Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @gilbens-starkware and @yuvalsw)

---

crates/cairo-lang-parser/src/parser_test.rs line 199 at r1 (raw file):

    },
    test_partial_parser_tree_with_trivia

Suggestion:

        path: "path",
        path_compat: "path_compat",
        attribute_errors: "attribute_errors",
    },
    test_partial_parser_tree_with_trivia

---

crates/cairo-lang-syntax-codegen/src/cairo_spec.rs line 667 at r1 (raw file):

    // An enum of all nodes that can be skipped while parsing.
    .add_enum(EnumBuilder::new("SkippedNodeInner")
        .node_with_explicit_kind("AttributeList", "AttributeList")

maybe we should have all the skipped types under the inner?
also - if you add the inner type to prevent the recursive enum - doc it here.

Code quote:

        .node_with_explicit_kind("AttributeList", "AttributeList")

[yuvalsw]

Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: all files reviewed, 18 unresolved discussions (waiting on @gilbens-starkware)

---

crates/cairo-lang-parser/src/parser.rs line 267 at r1 (raw file):

                if has_attrs {
                    Ok(self.skip_node_and_return_missing::<Item>(
                        SkippedNode::new_green(self.db, attributes.into()),

better if you can spare this by accepting "impl Into" as the parameter, and add the SkippedNode::new_green in the function

Code quote:

SkippedNode::new_green

---

crates/cairo-lang-parser/src/parser.rs line 2189 at r1 (raw file):

    /// Skips a node. A skipped node is a node which is not expected where it is found. Skipping
    /// this node means reporting an error, appending it to the current trivia, and continuing the

Please also add this part (about appending to the trivia) to the skip_token doc.

Suggestion:

, appending the node to the current trivia as skipped, and c

---

crates/cairo-lang-parser/src/parser.rs line 2191 at r1 (raw file):

    /// this node means reporting an error, appending it to the current trivia, and continuing the
    /// compilation as if it wasn't there.
    fn skip_node(&mut self, node_to_skip: SkippedNodeGreen, diagnostic_kind: ParserDiagnosticKind) {

Also please clarify this in the doc - as opposed to skip_token, here the node to skip is already fully taken/consumed.

Suggestion:

fn skip_taken_node(

---

crates/cairo-lang-parser/src/parser.rs line 2193 at r1 (raw file):

    fn skip_node(&mut self, node_to_skip: SkippedNodeGreen, diagnostic_kind: ParserDiagnosticKind) {
        // Add to pending trivia.
        self.pending_trivia.push(node_to_skip.into());

could the pending trivia be nonempty? Could it contain skipped tokens/nodes? I think that if we had trivia, it should have been attached to something inside the current node we are skipping.
I am not saying this means this/any code should change, but I think it's worth to thoroughly cover all the cases and document what's possible and why. This may float potential issues we haven't thought of (which I hope there aren't), e.g. a skipped token appearing twice in the tree, duplicate diagnostics etc.

For all the edge cases that come up from this thinking, try to add a test.

A simple test case is a trivia and then a skipped node:
mod a {
// comment
#[attr]
}

And a skipped token before a skipped node:
mod a {
$
#[attr]
}

Or after (not sure what's supposed to happen here):
mod a {
#[attr]
$
}

Assuming the '$' above is skipped, maybe also have:
mod a {
#[attr]
$
#[attr]
}

---

crates/cairo-lang-parser/src/parser.rs line 2193 at r1 (raw file):

    fn skip_node(&mut self, node_to_skip: SkippedNodeGreen, diagnostic_kind: ParserDiagnosticKind) {
        // Add to pending trivia.
        self.pending_trivia.push(node_to_skip.into());

Why did you do it in a different order than in append\_skipped\_token\_to\_pending\_trivia?
If there is no good reason, better for both to be in the same logical order for ease of reading.

Code quote:

        // Add to pending trivia.
        self.pending_trivia.push(node_to_skip.into());

---

crates/cairo-lang-parser/src/parser.rs line 2202 at r1 (raw file):

            span: TextSpan { start: diag_start, end: diag_end },
            leading_trivia_start: orig_offset,
            trailing_trivia_end: diag_end,

As in append\_skipped\_token\_to\_pending\_trivia, I think you should add here the width of the trailing trivia. IIRC it was added to correctly merge skip diagnostics (this way we can see that 2 consecutive ones are "touching").

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 0 at r1 (raw file):
In all the tests - where are the "skipped" diagnostics for the orphan attributes?

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 55 at r1 (raw file):

//! > ignored_kinds
Attribute

Same in the impl item test below.

Suggestion:

//! > top_level_kind
TraitBody

//! > ignored_kinds
Attribute
TerminalLBrace
TerminalEndOfFile

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 58 at r1 (raw file):

//! > expected_diagnostics
error: Missing tokens. Expected an item after attributes.

This used to be "Expected a trait item ...". Can we get it back?
Same in the impl.

Code quote:

Expected an item after attributes.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 231 at r1 (raw file):

//! > ==========================================================================

//! > Test repeated attributed literals.

can you please rephrase?

Code quote:

Test repeated attributed literals.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 245 at r1 (raw file):

//! > ignored_kinds
FunctionDeclaration

Suggestion:

//! > top_level_kind
FunctionWithBody

//! > ignored_kinds
FunctionDeclaration
ExprBlock
TerminalLBrack
TerminalRBrack
ExprPath
TerminalHash

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 330 at r1 (raw file):

//! > ==========================================================================

//! > Test skipped inside a module.

Suggestion:

Test skipped node inside a module.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 344 at r1 (raw file):

//! > top_level_kind

//! > ignored_kinds

same as above, reduce to only the necessary part of the tree.

Code quote:

//! > top_level_kind

//! > ignored_kinds

---

crates/cairo-lang-syntax-codegen/src/cairo_spec.rs line 13 at r1 (raw file):

            .node_with_explicit_kind("Newline", "TokenNewline")
            .node_with_explicit_kind("Skipped", "TokenSkipped")
            .node_with_explicit_kind("SkippedNode", "SkippedNode"),

Then define TriviumSkippedNode below, and SkippedNodeInner can become SkippedNode (if indeed required as Ori commented).

Suggestion:

.node("SkippedNode"),

---

crates/cairo-lang-syntax-codegen/src/cairo_spec.rs line 664 at r1 (raw file):

    .add_struct(StructBuilder::new("SkippedNode")
    .node("node", "SkippedNodeInner")
    )

Suggestion:

    .add_struct(StructBuilder::new("SkippedNode")
        .node("node", "SkippedNodeInner")
    )

---

crates/cairo-lang-syntax-codegen/src/cairo_spec.rs line 665 at r1 (raw file):

    .node("node", "SkippedNodeInner")
    )
    // An enum of all nodes that can be skipped while parsing.

Please comment that it's OK to add kinds here later as needed. Theoretically we could add now all types, but didn't do it to avoid unnecessary code + possible confusion.

[gilbens-starkware]

Reviewable status: 2 of 10 files reviewed, 15 unresolved discussions (waiting on @orizi and @yuvalsw)

---

crates/cairo-lang-parser/src/parser.rs line 267 at r1 (raw file):

Previously, yuvalsw wrote…

better if you can spare this by accepting "impl Into" as the parameter, and add the SkippedNode::new_green in the function

Not sure I understand, what will be the type of the node passed to the function? AttributeList?

---

crates/cairo-lang-parser/src/parser.rs line 2191 at r1 (raw file):

Previously, yuvalsw wrote…

Also please clarify this in the doc - as opposed to skip_token, here the node to skip is already fully taken/consumed.

Done.

---

crates/cairo-lang-parser/src/parser.rs line 2193 at r1 (raw file):

Previously, yuvalsw wrote…

Why did you do it in a different order than in append\_skipped\_token\_to\_pending\_trivia?
If there is no good reason, better for both to be in the same logical order for ease of reading.

No reason, guess that I flipped the order in one of the edits.

---

crates/cairo-lang-parser/src/parser.rs line 2202 at r1 (raw file):

Previously, yuvalsw wrote…

As in append\_skipped\_token\_to\_pending\_trivia, I think you should add here the width of the trailing trivia. IIRC it was added to correctly merge skip diagnostics (this way we can see that 2 consecutive ones are "touching").

I got a small problem with it crossing the EOF, checking.

---

crates/cairo-lang-parser/src/parser_test.rs line 199 at r1 (raw file):

    },
    test_partial_parser_tree_with_trivia

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 55 at r1 (raw file):

Previously, yuvalsw wrote…

Same in the impl item test below.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 58 at r1 (raw file):

Previously, yuvalsw wrote…

This used to be "Expected a trait item ...". Can we get it back?
Same in the impl.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 231 at r1 (raw file):

Previously, yuvalsw wrote…

can you please rephrase?

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 245 at r1 (raw file):

//! > ignored_kinds
FunctionDeclaration

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 344 at r1 (raw file):

Previously, yuvalsw wrote…

same as above, reduce to only the necessary part of the tree.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line at r1 (raw file):

Previously, yuvalsw wrote…

In all the tests - where are the "skipped" diagnostics for the orphan attributes?

The passed diagnostic is to the skip function is "Missing item", I think it's what the user need to know, do we want two diagnostics?

---

crates/cairo-lang-syntax-codegen/src/cairo_spec.rs line 13 at r1 (raw file):

Previously, yuvalsw wrote…

Then define TriviumSkippedNode below, and SkippedNodeInner can become SkippedNode (if indeed required as Ori commented).

Done.

---

crates/cairo-lang-syntax-codegen/src/cairo_spec.rs line 665 at r1 (raw file):

Previously, yuvalsw wrote…

Please comment that it's OK to add kinds here later as needed. Theoretically we could add now all types, but didn't do it to avoid unnecessary code + possible confusion.

Done.
I don't want to add all nodes now without something that will generate it/test all nodes are there since if it will be manual there is a big chance it won't be maintained. If I'll add such test it'll be in a separate PR.

---

crates/cairo-lang-syntax-codegen/src/cairo_spec.rs line 667 at r1 (raw file):

Previously, orizi wrote…

maybe we should have all the skipped types under the inner?
also - if you add the inner type to prevent the recursive enum - doc it here.

About all of them, see my answer to Yuval.
Doc'd it. Just to make sure, when you say recursive enum you mean that an enum can't have an enum variant (any enum, not just itself), right?

[orizi]

Reviewed 2 of 8 files at r2, all commit messages.
Reviewable status: 4 of 10 files reviewed, 13 unresolved discussions (waiting on @gilbens-starkware and @yuvalsw)

[gilbens-starkware]

Reviewable status: 4 of 10 files reviewed, 13 unresolved discussions (waiting on @orizi and @yuvalsw)

---

crates/cairo-lang-parser/src/parser.rs line 2193 at r1 (raw file):

Previously, yuvalsw wrote…

could the pending trivia be nonempty? Could it contain skipped tokens/nodes? I think that if we had trivia, it should have been attached to something inside the current node we are skipping.
I am not saying this means this/any code should change, but I think it's worth to thoroughly cover all the cases and document what's possible and why. This may float potential issues we haven't thought of (which I hope there aren't), e.g. a skipped token appearing twice in the tree, duplicate diagnostics etc.

For all the edge cases that come up from this thinking, try to add a test.

A simple test case is a trivia and then a skipped node:
mod a {
// comment
#[attr]
}

And a skipped token before a skipped node:
mod a {
$
#[attr]
}

Or after (not sure what's supposed to happen here):
mod a {
#[attr]
$
}

Assuming the '$' above is skipped, maybe also have:
mod a {
#[attr]
$
#[attr]
}

Done.

[yuvalsw]

Reviewed 7 of 8 files at r2, all commit messages.
Reviewable status: 9 of 10 files reviewed, 16 unresolved discussions (waiting on @gilbens-starkware and @orizi)

a discussion (no related file):
I think you need to add a semantic test with an inline module with attrs without item, and then some diagnostic after the inline module, to see that it points to the right locations.

---

---

crates/cairo-lang-parser/src/parser.rs line 267 at r1 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Not sure I understand, what will be the type of the node passed to the function? AttributeList?

impl Into<SkippedNodeGreen> (which is currently only AttributeListGreen). I think it could work.

---

crates/cairo-lang-parser/src/parser.rs line 2193 at r1 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Done.

Can you elaborate?

---

crates/cairo-lang-parser/src/parser.rs line 2417 at r2 (raw file):

    let node = db.lookup_intern_green(green_id);
    if node.kind == SyntaxKind::Trivia {
        println!("trivia width: {:?}", node.width());

remove

---

crates/cairo-lang-parser/src/parser.rs line 2426 at r2 (raw file):

                leading_trivia_width(db, *child)
            } else {
                TextWidth::default()

is this case possible? If so, please doc how.

---

crates/cairo-lang-parser/src/parser.rs line 2433 at r2 (raw file):

/// The width of the trailing trivia, traversing the tree to the bottom right node.
pub fn trailing_trivia_width(db: &dyn SyntaxGroup, green_id: GreenId) -> TextWidth {

same comments as in leading

---

crates/cairo-lang-parser/src/parser.rs line 2433 at r2 (raw file):

/// The width of the trailing trivia, traversing the tree to the bottom right node.
pub fn trailing_trivia_width(db: &dyn SyntaxGroup, green_id: GreenId) -> TextWidth {

can you unite this function with a callback parameter for first/last child?

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 39 at r1 (raw file):

        └── trailing_trivia (kind: Trivia) []

//! > ==========================================================================

where are all the tests gone?

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line at r1 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

The passed diagnostic is to the skip function is "Missing item", I think it's what the user need to know, do we want two diagnostics?

I think so.

1. I think this is what we do for missing tokens in skip_token_and_return_missing, so we may want to be consistent.
2. The skipped tokens diagnostic is mainly informative - there is a missing item (this is the missing diag), but I (the compiler/parser) ignored/skipped the attributes in order to keep parsing the rest of the file.

I am not locked on this, so if you don't agree please let's discuss.

---

crates/cairo-lang-syntax-codegen/src/cairo_spec.rs line 665 at r1 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Done.
I don't want to add all nodes now without something that will generate it/test all nodes are there since if it will be manual there is a big chance it won't be maintained. If I'll add such test it'll be in a separate PR.

No, this is completely fine. Just resonated my request.

---

crates/cairo-lang-syntax-codegen/src/cairo_spec.rs line 665 at r2 (raw file):

    .add_struct(StructBuilder::new("TriviumSkippedNode")
    .node("node", "SkippedNode")
    )

Suggestion:

    .add_struct(StructBuilder::new("TriviumSkippedNode")
        .node("node", "SkippedNode")
    )

[gilbens-starkware]

Reviewable status: 9 of 10 files reviewed, 16 unresolved discussions (waiting on @orizi and @yuvalsw)

---

crates/cairo-lang-parser/src/parser.rs line 267 at r1 (raw file):

Previously, yuvalsw wrote…

impl Into<SkippedNodeGreen> (which is currently only AttributeListGreen). I think it could work.

Yes, that's the impl, but the node that'll be turned into SkippedNodeGreen, a generic type?

[gilbens-starkware]

Reviewable status: 9 of 10 files reviewed, 14 unresolved discussions (waiting on @orizi and @yuvalsw)

a discussion (no related file):

Previously, yuvalsw wrote…

I think you need to add a semantic test with an inline module with attrs without item, and then some diagnostic after the inline module, to see that it points to the right locations.

Done.

---

---

crates/cairo-lang-parser/src/parser.rs line 2193 at r1 (raw file):

Previously, yuvalsw wrote…

Can you elaborate?

Wrong push, see below.

---

crates/cairo-lang-parser/src/parser.rs line 2426 at r2 (raw file):

Previously, yuvalsw wrote…

is this case possible? If so, please doc how.

Done.

---

crates/cairo-lang-parser/src/parser.rs line 2433 at r2 (raw file):

Previously, yuvalsw wrote…

same comments as in leading

Done.

---

crates/cairo-lang-parser/src/parser.rs line 2433 at r2 (raw file):

Previously, yuvalsw wrote…

can you unite this function with a callback parameter for first/last child?

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 39 at r1 (raw file):

Previously, yuvalsw wrote…

where are all the tests gone?

Accidentally pushed a WIP version when I debugged.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line at r1 (raw file):

Previously, yuvalsw wrote…

I think so.

1. I think this is what we do for missing tokens in skip_token_and_return_missing, so we may want to be consistent.
2. The skipped tokens diagnostic is mainly informative - there is a missing item (this is the missing diag), but I (the compiler/parser) ignored/skipped the attributes in order to keep parsing the rest of the file.

I am not locked on this, so if you don't agree please let's discuss.

I don't really like it since the skip tokens diagnostics also mentions what was expected, it is good when we skip because the token is not expected at this point, it is not that good when the something is missing.
I made this change, let me know what you think.

[yuvalsw]

Reviewed 4 of 5 files at r3, all commit messages.
Reviewable status: 10 of 11 files reviewed, 26 unresolved discussions (waiting on @gilbens-starkware)

---

crates/cairo-lang-parser/src/parser.rs line 2196 at r3 (raw file):

    /// error, appending the node to the current trivia as skipped, and continuing the
    /// compilation as if it wasn't there.
    fn skip_taken_node(

haven't reviewed this function yet, will review tomorrow. Posting the rest for now.

---

crates/cairo-lang-parser/src/parser.rs line 2434 at r3 (raw file):

/// The width of the trailing trivia, traversing the tree to the bottom right node.
pub fn trailing_trivia_width(db: &dyn SyntaxGroup, green_id: GreenId) -> TextWidth {
    get_trivia_width(db, green_id, |children| children.last())

I think this should work

Suggestion:

id, Vec::last)

---

crates/cairo-lang-parser/src/parser.rs line 2437 at r3 (raw file):

}

pub fn get_trivia_width(

doc, specifically node_selector.

---

crates/cairo-lang-parser/src/parser.rs line 2452 at r3 (raw file):

                get_trivia_width(db, *child, node_selector)
            } else {
                unreachable!("Only tokens should have no children.")

Suggestion:

unreachable!("Only tokens should have no children.");

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 245 at r1 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Done.

Add top level kind to be FunctionWithBody and ExprPath to ignored kinds.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 330 at r1 (raw file):

//! > ==========================================================================

//! > Test skipped inside a module.

I don't see the change.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line at r1 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

I don't really like it since the skip tokens diagnostics also mentions what was expected, it is good when we skip because the token is not expected at this point, it is not that good when the something is missing.
I made this change, let me know what you think.

Let's think about it f2f.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 293 at r3 (raw file):

    ^****^

//! > expected_tree

I don't see the skipped node here.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 313 at r3 (raw file):

//! > cairo_code
mod mod1{

Suggestion:

1 {

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 340 at r3 (raw file):

//! > expected_tree
└── Top level kind: ModuleBody
    ├── lbrace (kind: TerminalLBrace)

can ignore

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 368 at r3 (raw file):

//! > cairo_code
mod mod1{

Suggestion:

od1 {

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 395 at r3 (raw file):

//! > expected_tree
└── Top level kind: ModuleBody
    ├── lbrace (kind: TerminalLBrace)

can ignore

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 454 at r3 (raw file):

//! > expected_tree
└── Top level kind: ModuleBody
    ├── lbrace (kind: TerminalLBrace)

can ignore

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 475 at r3 (raw file):

        │   └── child #3 (kind: TokenNewline).
        ├── token (kind: TokenRBrace): '}'
        └── trailing_trivia (kind: Trivia) []

I don't see the first comment here.
It may be in the trivia of the TerminalHash? If so, don't ignore it.

Code quote:

    └── rbrace (kind: TerminalRBrace)
        ├── leading_trivia (kind: Trivia)
        │   ├── child #0 (kind: TriviumSkippedNode)
        │   │   └── node (kind: AttributeList)
        │   │       └── child #0 (kind: Attribute)
        │   │           ├── hash (kind: TerminalHash) <ignored>
        │   │           ├── lbrack (kind: TerminalLBrack) <ignored>
        │   │           ├── attr (kind: ExprPath) <ignored>
        │   │           ├── arguments (kind: OptionArgListParenthesizedEmpty) []
        │   │           └── rbrack (kind: TerminalRBrack) <ignored>
        │   ├── child #1 (kind: TokenWhitespace).
        │   ├── child #2 (kind: TokenSingleLineComment): '// A second comment'
        │   └── child #3 (kind: TokenNewline).
        ├── token (kind: TokenRBrace): '}'
        └── trailing_trivia (kind: Trivia) []

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 513 at r3 (raw file):

 --> dummy_file.cairo:3:5
    #[aaa]
    ^****^

It would be good if the 2 skipped diags would be merged. Why don't they?

Code quote:

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro or an attribute.
 --> dummy_file.cairo:2:5
    $
    ^

error: Missing tokens. Expected an item after attributes.
 --> dummy_file.cairo:3:11
    #[aaa]
          ^

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro.
 --> dummy_file.cairo:3:5
    #[aaa]
    ^****^

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 517 at r3 (raw file):

//! > expected_tree
└── Top level kind: ModuleBody
    ├── lbrace (kind: TerminalLBrace)

can ignore

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 535 at r3 (raw file):

        │               └── rbrack (kind: TerminalRBrack) <ignored>
        ├── token (kind: TokenRBrace): '}'
        └── trailing_trivia (kind: Trivia) []

make the $ visible here

Code quote:

    └── rbrace (kind: TerminalRBrace)
        ├── leading_trivia (kind: Trivia)
        │   └── child #0 (kind: TriviumSkippedNode)
        │       └── node (kind: AttributeList)
        │           └── child #0 (kind: Attribute)
        │               ├── hash (kind: TerminalHash) <ignored>
        │               ├── lbrack (kind: TerminalLBrack) <ignored>
        │               ├── attr (kind: ExprPath) <ignored>
        │               ├── arguments (kind: OptionArgListParenthesizedEmpty) []
        │               └── rbrack (kind: TerminalRBrack) <ignored>
        ├── token (kind: TokenRBrace): '}'
        └── trailing_trivia (kind: Trivia) []

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 572 at r3 (raw file):

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro or an attribute.
 --> dummy_file.cairo:3:5
    $

same - would be good to merge.

Code quote:

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro.
 --> dummy_file.cairo:2:5
    #[aaa]
    ^****^

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro or an attribute.
 --> dummy_file.cairo:3:5
    $

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 577 at r3 (raw file):

//! > expected_tree
└── Top level kind: ModuleBody
    ├── lbrace (kind: TerminalLBrace)

can ignore

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 647 at r3 (raw file):

 --> dummy_file.cairo:4:5
    #[bbb]
    ^****^

same - merge skipped diags

Code quote:

error: Missing tokens. Expected an item after attributes.
 --> dummy_file.cairo:2:11
    #[aaa]
          ^

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro.
 --> dummy_file.cairo:2:5
    #[aaa]
    ^****^

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro or an attribute.
 --> dummy_file.cairo:3:5
    $
    ^

error: Missing tokens. Expected an item after attributes.
 --> dummy_file.cairo:4:11
    #[bbb]
          ^

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro.
 --> dummy_file.cairo:4:5
    #[bbb]
    ^****^

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 651 at r3 (raw file):

//! > expected_tree
└── Top level kind: ModuleBody
    ├── lbrace (kind: TerminalLBrace)

can ignore

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 661 at r3 (raw file):

    └── rbrace (kind: TerminalRBrace)
        ├── leading_trivia (kind: Trivia)
        │   └── child #0 (kind: TriviumSkippedNode)

I want to see in the tree: twice a TriviumSkippedNode and once a "$",

Code quote:

TriviumSkippedNode

---

crates/cairo-lang-semantic/src/diagnostic_test_data/tests line 243 at r3 (raw file):

//! > module_code
mod mod1{

Suggestion:

mod1 {

[gilbens-starkware]

Reviewable status: 10 of 11 files reviewed, 26 unresolved discussions (waiting on @yuvalsw)

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 513 at r3 (raw file):

Previously, yuvalsw wrote…

It would be good if the 2 skipped diags would be merged. Why don't they?

The kind wasn't exactly the same, as I haven't added "or an attribute", which is debatable (another attribute is valid after the skipped attribute, but at some point we'll need an item.
Changed it, but we should be aware that diagnostics spanning more than one line are bad.

[yuvalsw]

Reviewed 1 of 3 files at r4, all commit messages.
Reviewable status: 9 of 11 files reviewed, 29 unresolved discussions (waiting on @gilbens-starkware)

---

crates/cairo-lang-parser/src/parser.rs line 2208 at r4 (raw file):

        // Remove the node width, as it was already taken, and fix for the last token not being
        // considered in the current offset.
        let orig_offset =

It's not orig_offset

Code quote:

orig_offset

---

crates/cairo-lang-parser/src/parser.rs line 2209 at r4 (raw file):

        // considered in the current offset.
        let orig_offset =
            self.offset.add_width(self.current_width).sub_width(node_to_skip.0.width(self.db));

this is calculated twice.

Code quote:

node_to_skip.0.width(self.db)

---

crates/cairo-lang-parser/src/parser.rs line 2217 at r4 (raw file):

        let diag_start = orig_offset.add_width(node_leading_trivia_width);
        // Position of the node before the trailing trivia.
        let diag_end = diag_start.add_width(width_without_trivia);

Suggestion:

    
        // Position of the node after the leading trivia.
        let diag_start = orig_offset.add_width(node_leading_trivia_width);
        // Position of the node before the trailing trivia.
        let diag_end = orig_offset.add_width(node_to_skip.0.width(self.db) - node_trailing_trivia_width);

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 130 at r3 (raw file):

 --> dummy_file.cairo:3:5
    3
    ^

why is this gone?

Code quote:

error: Skipped tokens. Expected: impl item or an attribute.
 --> dummy_file.cairo:3:5
    3
    ^

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 228 at r3 (raw file):

^****^

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro or an attribute.

and this

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 513 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

The kind wasn't exactly the same, as I haven't added "or an attribute", which is debatable (another attribute is valid after the skipped attribute, but at some point we'll need an item.
Changed it, but we should be aware that diagnostics spanning more than one line are bad.

We need to fix multi line diags. But I don't see the skipped diags being merged.

[gilbens-starkware]

Reviewable status: 7 of 11 files reviewed, 25 unresolved discussions (waiting on @yuvalsw)

---

crates/cairo-lang-parser/src/parser.rs line 267 at r1 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Yes, that's the impl, but the node that'll be turned into SkippedNodeGreen, a generic type?

Done

---

crates/cairo-lang-parser/src/parser.rs line 2202 at r1 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

I got a small problem with it crossing the EOF, checking.

Done.

---

crates/cairo-lang-parser/src/parser.rs line 2434 at r3 (raw file):

Previously, yuvalsw wrote…

I think this should work

last is a method of slice, and it doesn't have a a literal type (the impl declaration is impl<T> [T]), so I can't really access it.
I tried making a type alias `type SliceType = [T]' just out of curiosity, but I got some weird errors, which I think are because each function should have only one function pointer but I haven't tried to fully understand it.

---

crates/cairo-lang-parser/src/parser.rs line 2437 at r3 (raw file):

Previously, yuvalsw wrote…

doc, specifically node_selector.

Done.

---

crates/cairo-lang-parser/src/parser.rs line 2208 at r4 (raw file):

Previously, yuvalsw wrote…

It's not orig_offset

Done.

---

crates/cairo-lang-parser/src/parser.rs line 2209 at r4 (raw file):

Previously, yuvalsw wrote…

this is calculated twice.

Done.

---

crates/cairo-lang-parser/src/parser.rs line 2217 at r4 (raw file):

        let diag_start = orig_offset.add_width(node_leading_trivia_width);
        // Position of the node before the trailing trivia.
        let diag_end = diag_start.add_width(width_without_trivia);

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 245 at r1 (raw file):

Previously, yuvalsw wrote…

Add top level kind to be FunctionWithBody and ExprPath to ignored kinds.

I'm not sure which test you are referring to.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line at r1 (raw file):

Previously, yuvalsw wrote…

Let's think about it f2f.

Returned to just Missing as discussed.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 130 at r3 (raw file):

Previously, yuvalsw wrote…

why is this gone?

It's merged, that's the biggest problem with the multiline diags, I think that now with skipped nodes we should prioritise fixing it.
Should be easy once we'll decide on the format.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 228 at r3 (raw file):

Previously, yuvalsw wrote…

and this

Same.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 293 at r3 (raw file):

Previously, yuvalsw wrote…

I don't see the skipped node here.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 340 at r3 (raw file):

Previously, yuvalsw wrote…

can ignore

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 395 at r3 (raw file):

Previously, yuvalsw wrote…

can ignore

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 454 at r3 (raw file):

Previously, yuvalsw wrote…

can ignore

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 475 at r3 (raw file):

Previously, yuvalsw wrote…

I don't see the first comment here.
It may be in the trivia of the TerminalHash? If so, don't ignore it.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 513 at r3 (raw file):

Previously, yuvalsw wrote…

We need to fix multi line diags. But I don't see the skipped diags being merged.

They are merged, but only the first line is shown.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 517 at r3 (raw file):

Previously, yuvalsw wrote…

can ignore

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 535 at r3 (raw file):

Previously, yuvalsw wrote…

make the $ visible here

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 572 at r3 (raw file):

Previously, yuvalsw wrote…

same - would be good to merge.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 577 at r3 (raw file):

Previously, yuvalsw wrote…

can ignore

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 647 at r3 (raw file):

Previously, yuvalsw wrote…

same - merge skipped diags

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 651 at r3 (raw file):

Previously, yuvalsw wrote…

can ignore

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 661 at r3 (raw file):

Previously, yuvalsw wrote…

I want to see in the tree: twice a TriviumSkippedNode and once a "$",

Done.

[yuvalsw]

Reviewed 4 of 4 files at r5, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @gilbens-starkware)

---

crates/cairo-lang-parser/src/parser.rs line 2238 at r5 (raw file):

    /// Skips a given SkippedNode, reports the given diagnostic, and a diagnostic for the skipped
    /// node, and returns missing kind of the expected node.

Suggestion:

    /// Skips a given SkippedNode, reports the given diagnostic and returns missing kind of the expected node.

---

crates/cairo-lang-parser/src/parser_test_data/full_trees/test1 line 41 at r3 (raw file):

                         ^

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro.

The previous pointer was better, pointing to after the attribute. Why did it change?

Code quote:

 --> src/parser_test_data/cairo_test_files/test1.cairo:62:26
#[attribute_without_item]
                         ^

error: Skipped tokens. Expected: Const/Module/Use/FreeFunction/ExternFunction/ExternType/Trait/Impl/Struct/Enum/TypeAlias/InlineMacro.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 245 at r1 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

I'm not sure which test you are referring to.

If you go to the revision I wrote it on originally (r1 in this case), you can see. Here I meant "Test attributes with skipeed braced element in between.",

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 130 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

It's merged, that's the biggest problem with the multiline diags, I think that now with skipped nodes we should prioritise fixing it.
Should be easy once we'll decide on the format.

Yeah, let's prioritise it.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 340 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Done.

I still see it.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 395 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Done.

I still see it

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 454 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Done.

I still see it

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 517 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Done.

I still see it.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 577 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Done.

I still see it

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 651 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Done.

I still see it

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 290 at r5 (raw file):

    ├── lbrace (kind: TerminalLBrace) <ignored>
    ├── items (kind: ItemList)
    │   ├── child #0 (kind: FunctionWithBody)

can ignore

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 660 at r5 (raw file):

        │               │   │   │           │   └── trailing_trivia (kind: Trivia) []
        │               │   │   │           ├── lbrack (kind: TerminalLBrack) <ignored>
        │               │   │   │           ├── attr (kind: ExprPath) <ignored>

Maybe don't ignore this so we at least see which attr it is. It doesn't save a lot of test anyway.

---

crates/cairo-lang-semantic/src/diagnostic_test_data/tests line 243 at r3 (raw file):

//! > module_code
mod mod1{

I don't see it.

[gilbens-starkware]

Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @yuvalsw)

---

crates/cairo-lang-parser/src/parser.rs line 2238 at r5 (raw file):

    /// Skips a given SkippedNode, reports the given diagnostic, and a diagnostic for the skipped
    /// node, and returns missing kind of the expected node.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/full_trees/test1 line 41 at r3 (raw file):

Previously, yuvalsw wrote…

The previous pointer was better, pointing to after the attribute. Why did it change?

Because it was not raised from the skipping function, if it's the diagnostic raised from the skipping then it should point to the skipped part, no?

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 245 at r1 (raw file):

Previously, yuvalsw wrote…

If you go to the revision I wrote it on originally (r1 in this case), you can see. Here I meant "Test attributes with skipeed braced element in between.",

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 340 at r3 (raw file):

Previously, yuvalsw wrote…

I still see it.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 395 at r3 (raw file):

Previously, yuvalsw wrote…

I still see it

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 454 at r3 (raw file):

Previously, yuvalsw wrote…

I still see it

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 517 at r3 (raw file):

Previously, yuvalsw wrote…

I still see it.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 577 at r3 (raw file):

Previously, yuvalsw wrote…

I still see it

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 651 at r3 (raw file):

Previously, yuvalsw wrote…

I still see it

Done.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 290 at r5 (raw file):

Previously, yuvalsw wrote…

can ignore

It'll hide the first attribute.

---

crates/cairo-lang-parser/src/parser_test_data/partial_trees_with_trivia/attribute_errors line 660 at r5 (raw file):

Previously, yuvalsw wrote…

Maybe don't ignore this so we at least see which attr it is. It doesn't save a lot of test anyway.

Done.

[yuvalsw]

Reviewed 3 of 3 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @gilbens-starkware)

---

crates/cairo-lang-parser/src/parser.rs line 2194 at r6 (raw file):

    /// Skips a given node, which is already taken, and can be transformed into a SkippedNode (i.e.
    /// is a variant of SkippedNode). A skipped node is a node which is not expected where it is

Suggestion:

    /// Skips the given node which is a variant of `SkippedNode` and is already taken. A skipped node is a node which is not expected where it is

---

crates/cairo-lang-parser/src/parser_test_data/full_trees/test1 line 41 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Because it was not raised from the skipping function, if it's the diagnostic raised from the skipping then it should point to the skipped part, no?

I don't follow. Also - it was ok before your PR, now it's not. The missing tokens diag should point to after the attr, where it's missing.

[gilbens-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yuvalsw)

---

crates/cairo-lang-parser/src/parser_test_data/full_trees/test1 line 41 at r3 (raw file):

Previously, yuvalsw wrote…

I don't follow. Also - it was ok before your PR, now it's not. The missing tokens diag should point to after the attr, where it's missing.

I changed the behaviour to point at the node.
Anyway, added a flag to support both options.

[yuvalsw]

Reviewed 4 of 4 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gilbens-starkware)

---

crates/cairo-lang-parser/src/parser.rs line 2216 at r7 (raw file):

        
        // Remove the node width, as it was already taken, and fix for the last token not being
        // considered in the current offset.

fix/split the comment.

Code quote:

        // Remove the node width, as it was already taken, and fix for the last token not being
        // considered in the current offset.

---

crates/cairo-lang-parser/src/parser.rs line 2221 at r7 (raw file):

        let diag_end = end_of_node_offset.sub_width(node_trailing_trivia_width);
        let diag_start = if diagnostic_pointing_at_node {
            let node_leading_trivia_width = leading_trivia_width(self.db, trivium_green.0);

inline

---

crates/cairo-lang-parser/src/parser_test_data/full_trees/test1 line 41 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

I changed the behaviour to point at the node.
Anyway, added a flag to support both options.

when should this flag be true?

[gilbens-starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yuvalsw)

---

crates/cairo-lang-parser/src/parser.rs line 2216 at r7 (raw file):

Previously, yuvalsw wrote…

fix/split the comment.

Done.

---

crates/cairo-lang-parser/src/parser_test_data/full_trees/test1 line 41 at r3 (raw file):

Previously, yuvalsw wrote…

when should this flag be true?

If we the diagnostic kind indicates that the node was skipped, but we don't report what is missing. I can remove it if you think so.

[yuvalsw]

Reviewed 1 of 1 files at r9, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware)

---

crates/cairo-lang-parser/src/parser_test_data/full_trees/test1 line 41 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

If we the diagnostic kind indicates that the node was skipped, but we don't report what is missing. I can remove it if you think so.

Yes, let's remove it. I don't know if we'll have such a case

[gilbens-starkware]

Reviewable status: 10 of 11 files reviewed, 1 unresolved discussion (waiting on @yuvalsw)

---

crates/cairo-lang-parser/src/parser_test_data/full_trees/test1 line 41 at r3 (raw file):

Previously, yuvalsw wrote…

Yes, let's remove it. I don't know if we'll have such a case

Done.

[yuvalsw]

Reviewed 1 of 1 files at r10, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware)

---

crates/cairo-lang-parser/src/parser.rs line 2197 at r10 (raw file):

    /// reporting an error, appending the node to the current trivia as skipped, and continuing
    /// the compilation as if it wasn't there. The diagnostic can either point to the node, or
    /// after it, depending on `diagnostic_pointing_at_node`.

Same for skip_taken_node_and_return_missing

Suggestion:

    /// Skips the given node which is a variant of `SkippedNode` and is already taken. A skipped
    /// node is a node which is not expected where it is found. Skipping this node means
    /// reporting the given error (pointing to right after the node), appending the node to the current trivia as skipped, and continuing
    /// the compilation as if it wasn't there. 

[gilbens-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yuvalsw)

---

crates/cairo-lang-parser/src/parser.rs line 2197 at r10 (raw file):

Previously, yuvalsw wrote…

Same for skip_taken_node_and_return_missing

Done.

[yuvalsw]

Reviewed 1 of 1 files at r11, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)

[yuvalsw]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)

[orizi]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)