Skip to content

ocamllex: Check for matched parentheses in actions#11716

Merged
damiendoligez merged 1 commit intoocaml:trunkfrom
DemiMarie:ocamllex-balanced-parens
Feb 7, 2024
Merged

ocamllex: Check for matched parentheses in actions#11716
damiendoligez merged 1 commit intoocaml:trunkfrom
DemiMarie:ocamllex-balanced-parens

Conversation

@DemiMarie
Copy link
Contributor

This checks that parentheses and curly braces are properly balenced in ocamllex actions.

Fixes #7455 for ocamllex (it was already fixed for ocamlyacc).

@DemiMarie DemiMarie force-pushed the ocamllex-balanced-parens branch 3 times, most recently from ccf4470 to b8b0599 Compare November 12, 2022 03:46
@shindere
Copy link
Contributor

shindere commented Nov 14, 2022 via email

@xavierleroy
Copy link
Contributor

+1 for a stack represented as a list. You don't even need a reference: the stack can be passed as argument to the action rule, as in (untested code):

and action stk = parse
    | '{'
        { action ('{' :: stk) lexbuf }
    | '}'
        { match stk with
          | [] -> Lexing.lexeme_start lexbuf
          | '{' :: stk' -> action stk' lexbuf
          |  c  :: _ -> raise_lexical_error lexbuf
                                 ("unmatched " ^ Char.escaped c)
        }

@DemiMarie DemiMarie force-pushed the ocamllex-balanced-parens branch from b8b0599 to 13273a8 Compare November 15, 2022 23:14
@DemiMarie
Copy link
Contributor Author

+1 for a stack represented as a list. You don't even need a reference: the stack can be passed as argument to the action rule, as in (untested code):

and action stk = parse
    | '{'
        { action ('{' :: stk) lexbuf }
    | '}'
        { match stk with
          | [] -> Lexing.lexeme_start lexbuf
          | '{' :: stk' -> action stk' lexbuf
          |  c  :: _ -> raise_lexical_error lexbuf
                                 ("unmatched " ^ Char.escaped c)
        }

Nice trick! I mostly write C and Rust nowadays, and had already implemented this for ocamlyacc, so I went with the imperative approach I was familiar with. I switched to the approach you suggested, which required refactoring other code to use a functional style as well.

@damiendoligez damiendoligez self-assigned this Jan 17, 2023
Copy link
Member

@damiendoligez damiendoligez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine. I left a couple of suggestions that you're free to follow or ignore, my approval stands either way.

@damiendoligez
Copy link
Member

@DemiMarie could you please rebase?

@DemiMarie DemiMarie force-pushed the ocamllex-balanced-parens branch from 13273a8 to 572fd80 Compare May 23, 2023 14:38
@DemiMarie
Copy link
Contributor Author

@damiendoligez done

@shindere
Copy link
Contributor

shindere commented Jul 19, 2023 via email

@shindere
Copy link
Contributor

shindere commented Jul 19, 2023 via email

@DemiMarie
Copy link
Contributor Author

Sorry, no. This branch now has conflicts with trunk so a rebase is required. Gentle ping to @DemiMarie to do so and perhaps you could take this opportunity to take into account @damiendoligez's suggestions?

I can do so but not immediately due to time constraints.

@damiendoligez
Copy link
Member

@DemiMarie We can merge this whenever you have the time to fix the trivial conflict in Changes.

This checks that parentheses and curly braces are properly balenced in
ocamllex actions.  It also significantly refactors ocamllex’s lexer to
pass state using function arguments instead of mutable global variables.

Fixes ocaml#7455 for ocamllex (it was already fixed for ocamlyacc).
@DemiMarie DemiMarie force-pushed the ocamllex-balanced-parens branch from 572fd80 to 761a1a9 Compare February 6, 2024 18:06
@DemiMarie
Copy link
Contributor Author

@DemiMarie We can merge this whenever you have the time to fix the trivial conflict in Changes.

Done.

@damiendoligez damiendoligez merged commit 403d2b5 into ocaml:trunk Feb 7, 2024
@damiendoligez
Copy link
Member

Merged at last! Many thanks for your work and your patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ocamllex and ocamlyacc should check that there are balanced parentheses in code blocks

4 participants