Skip to content
This repository has been archived by the owner on Jun 3, 2021. It is now read-only.

Separate semantic analysis from parsing #151

Closed
jyn514 opened this issue Dec 10, 2019 · 3 comments
Closed

Separate semantic analysis from parsing #151

jyn514 opened this issue Dec 10, 2019 · 3 comments
Assignees
Labels
enhancement New feature or request hard Extra attention is needed parser Issue to do with parsing the abstract syntax tree technical-debt

Comments

@jyn514
Copy link
Owner

jyn514 commented Dec 10, 2019

I've meant to do this for a while. Code like this is just horrifying:

        self.left_associative_binary_op(next_grammar_func, tokens, |expr, next, token| {
            let non_scalar = if !expr.ctype.is_integral() {
                Some(&expr.ctype)
            } else if !next.ctype.is_integral() {
                Some(&next.ctype)
            } else {
                None
            };
            if let Some(ctype) = non_scalar {
                return Err((
                    Locatable {
                        data: format!(
                            "expected integer on both sides of '{}', got '{}'",
                            token.data, ctype
                        ),
                        location: token.location,
                    },
                    *expr,
                ));
            }
            let (promoted_expr, next) = Expr::binary_promote(*expr, *next).map_err(flatten)?;
            expr_func(promoted_expr, next, token)
        })

It obscures what the function's actually doing and also makes the error handling really complicated. Type checking should go in a separate pass.

@jyn514 jyn514 added enhancement New feature or request technical-debt parser Issue to do with parsing the abstract syntax tree labels Dec 10, 2019
@jyn514 jyn514 self-assigned this Dec 10, 2019
@jyn514
Copy link
Owner Author

jyn514 commented Dec 10, 2019

Things this would improve:

  • stack size in the heavily recursive part of the parser would go down by a lot
  • the code would hopefully be easier to read/more modular
  • the code would be more reusable (e.g. Binary promotion for complex assignment #25)
  • hopefully left_associative_binary_op could be refactored to not need to take a closure

@jyn514
Copy link
Owner Author

jyn514 commented Dec 10, 2019

Made a start on this in https://github.com/jyn514/rcc/tree/semantic-analysis and it's going to be even more work than I thought :(

@jyn514 jyn514 pinned this issue Dec 17, 2019
@jyn514
Copy link
Owner Author

jyn514 commented Mar 3, 2020

Work on this is ongoing in https://github.com/jyn514/rcc/tree/pratt-parsing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request hard Extra attention is needed parser Issue to do with parsing the abstract syntax tree technical-debt
Projects
None yet
Development

No branches or pull requests

1 participant