-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change ParseOutput
to return a NonTerminal
instead of a Node
#1187
base: main
Are you sure you want to change the base?
Changes from 16 commits
5db05e8
86540f4
a74ea7d
cf6b964
3fbefa4
24aff8c
ecd5062
4113609
7495413
4dc2d0b
8983d3c
72d4858
26d5e00
160e02e
95137a0
e8c1398
05d405a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -1,6 +1,6 @@ | ||||
use std::rc::Rc; | ||||
|
||||
use crate::cst::{Edge, Node, TerminalKind, TerminalKindExtensions, TextIndex}; | ||||
use crate::cst::{Edge, Node, NonterminalNode, TerminalKind, TerminalKindExtensions, TextIndex}; | ||||
use crate::parser::lexer::Lexer; | ||||
use crate::parser::parser_support::context::ParserContext; | ||||
use crate::parser::parser_support::parser_result::{ | ||||
|
@@ -85,14 +85,9 @@ where | |||
TerminalKind::UNRECOGNIZED | ||||
}; | ||||
let node = Node::terminal(kind, input.to_string()); | ||||
let tree = if no_match.kind.is_none() || start.utf8 == 0 { | ||||
node | ||||
} else { | ||||
trivia_nodes.push(Edge::anonymous(node)); | ||||
Node::nonterminal(no_match.kind.unwrap(), trivia_nodes) | ||||
}; | ||||
trivia_nodes.push(Edge::anonymous(node)); | ||||
ParseOutput { | ||||
parse_tree: tree, | ||||
parse_tree: Rc::new(NonterminalNode::new(no_match.kind.unwrap(), trivia_nodes)), | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we are always unwrapping There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is an example of how it's used today: slang/crates/codegen/runtime/generator/src/parser/codegen/precedence_parser_definition.rs Line 45 in f257eae
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC, in the case you mentioned, the kind is already known ( |
||||
errors: vec![ParseError::new( | ||||
start..start + input.into(), | ||||
no_match.expected_terminals, | ||||
|
@@ -156,18 +151,17 @@ where | |||
)); | ||||
|
||||
ParseOutput { | ||||
parse_tree: Node::nonterminal(topmost_node.kind, new_children), | ||||
parse_tree: Rc::new(NonterminalNode::new(topmost_node.kind, new_children)), | ||||
errors, | ||||
} | ||||
} else { | ||||
let parse_tree = Node::Nonterminal(topmost_node); | ||||
let parse_tree = topmost_node; | ||||
let errors = stream.into_errors(); | ||||
|
||||
// Sanity check: Make sure that succesful parse is equivalent to not having any invalid nodes | ||||
debug_assert_eq!( | ||||
errors.is_empty(), | ||||
parse_tree | ||||
.clone() | ||||
Rc::clone(&parse_tree) | ||||
.cursor_with_offset(TextIndex::ZERO) | ||||
.remaining_nodes() | ||||
.all(|edge| edge | ||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TOL: I wonder why cloning by default? WDYT of returning
&Rc<>
and leave cloning up to the user? This is similar to other APIs likeNode::as_nonterminal()
.I realize that
Rc::clone()
is verbose, but we decided to enable a lint rule that enforces that for clarity (we can revisit this decision separately).