Skip to content

Commit 6fc2aa5

Browse files
committed
WIP: Flip the ControlFlow/Result in the visitor
This makes the control flow in the visitor more explicit at the price of not being able to use Result propagation automatically in the Visitor implementations.
1 parent c35ba23 commit 6fc2aa5

5 files changed

Lines changed: 87 additions & 87 deletions

File tree

crates/codegen/parser/runtime/src/visitor.rs

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,22 @@ pub trait Visitor<E> {
1212
&mut self,
1313
_node: &Rc<RuleNode>,
1414
_cursor: &Cursor,
15-
) -> Result<ControlFlow<(), Step>, E> {
16-
Ok(ControlFlow::Continue(Step::In))
15+
) -> ControlFlow<Result<(), E>, Step> {
16+
ControlFlow::Continue(Step::In)
1717
}
1818

1919
/// Called when the [`Visitor`] exits a [`RuleNode`].
20-
fn rule_exit(&mut self, _node: &Rc<RuleNode>, _cursor: &Cursor) -> Result<ControlFlow<()>, E> {
21-
Ok(ControlFlow::Continue(()))
20+
fn rule_exit(
21+
&mut self,
22+
_node: &Rc<RuleNode>,
23+
_cursor: &Cursor,
24+
) -> ControlFlow<Result<(), E>, ()> {
25+
ControlFlow::Continue(())
2226
}
2327

2428
/// Called when the [`Visitor`] enters a [`TokenNode`].
25-
fn token(&mut self, _node: &Rc<TokenNode>, _cursor: &Cursor) -> Result<ControlFlow<()>, E> {
26-
Ok(ControlFlow::Continue(()))
29+
fn token(&mut self, _node: &Rc<TokenNode>, _cursor: &Cursor) -> ControlFlow<Result<(), E>, ()> {
30+
ControlFlow::Continue(())
2731
}
2832
}
2933

@@ -37,39 +41,32 @@ impl Cursor {
3741
pub fn drive_visitor<E, V: Visitor<E>>(
3842
&mut self,
3943
visitor: &mut V,
40-
) -> Result<ControlFlow<()>, E> {
44+
) -> ControlFlow<Result<(), E>, Step> {
4145
if self.is_completed() {
42-
return Ok(ControlFlow::Continue(()));
46+
return ControlFlow::Break(Ok(()));
4347
}
4448

4549
loop {
4650
// Node clone is cheap because it's just an enum around an Rc
4751
match self.node() {
48-
Node::Rule(rule_node) => {
49-
match visitor.rule_enter(&rule_node, self)? {
50-
ControlFlow::Break(()) => return Ok(ControlFlow::Break(())),
51-
ControlFlow::Continue(Step::In) => {
52+
Node::Rule(node) => {
53+
match visitor.rule_enter(&node, self)? {
54+
Step::Over => {}
55+
Step::In => {
5256
if self.go_to_first_child() {
5357
self.drive_visitor(visitor)?;
5458
self.go_to_parent();
5559
}
5660
}
57-
ControlFlow::Continue(Step::Over) => {}
58-
}
59-
if visitor.rule_exit(&rule_node, self)? == ControlFlow::Break(()) {
60-
return Ok(ControlFlow::Break(()));
6161
}
62+
visitor.rule_exit(&node, self)?;
6263
}
6364

64-
Node::Token(token_node) => {
65-
if visitor.token(&token_node, self)? == ControlFlow::Break(()) {
66-
return Ok(ControlFlow::Break(()));
67-
}
68-
}
65+
Node::Token(node) => visitor.token(&node, self)?,
6966
}
7067

7168
if !self.go_to_next_sibling() {
72-
return Ok(ControlFlow::Continue(()));
69+
return ControlFlow::Break(Ok(()));
7370
}
7471
}
7572
}

crates/solidity/outputs/cargo/crate/src/generated/visitor.rs

Lines changed: 19 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/solidity/outputs/cargo/tests/src/doc_examples/visitor_api.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::ops::ControlFlow;
22
use std::rc::Rc;
33

4-
use anyhow::{bail, ensure, Error, Result};
4+
use anyhow::{anyhow, Error, Result};
55
use semver::Version;
66

77
use slang_solidity::{
@@ -21,19 +21,23 @@ impl Visitor<Error> for ContractCollector {
2121
&mut self,
2222
node: &Rc<RuleNode>,
2323
_cursor: &Cursor,
24-
) -> Result<ControlFlow<(), Step>> {
24+
) -> ControlFlow<Result<(), Error>, Step> {
2525
if node.kind == RuleKind::ContractDefinition {
26-
if let Node::Token(token) = &node.children[2] {
27-
ensure!(token.kind == TokenKind::Identifier);
28-
self.contract_names.push(token.text.to_owned());
29-
} else {
30-
bail!("Expected contract identifier: {node:?}");
31-
};
32-
33-
return Ok(ControlFlow::Continue(Step::Over));
26+
match &node.children[2] {
27+
Node::Token(token) if token.kind == TokenKind::Identifier => {
28+
self.contract_names.push(token.text.to_owned());
29+
}
30+
_ => {
31+
return ControlFlow::Break(Err(anyhow!(
32+
"Expected contract identifier: {node:?}"
33+
)))
34+
}
35+
}
36+
37+
return ControlFlow::Continue(Step::Over);
3438
}
3539

36-
Ok(ControlFlow::Continue(Step::In))
40+
ControlFlow::Continue(Step::In)
3741
}
3842
}
3943

@@ -46,10 +50,13 @@ fn visitor_api() -> Result<()> {
4650
contract_names: Vec::new(),
4751
};
4852

49-
parse_output
53+
if let ControlFlow::Break(result) = parse_output
5054
.parse_tree()
5155
.cursor()
52-
.drive_visitor(&mut collector)?;
56+
.drive_visitor(&mut collector)
57+
{
58+
result?;
59+
}
5360

5461
assert!(matches!(&collector.contract_names[..], [single] if single == "Foo"));
5562

crates/solidity/outputs/npm/crate/src/generated/visitor.rs

Lines changed: 19 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/solidity/testing/utils/src/cst_snapshots/test_nodes.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,17 @@ impl Visitor<()> for TestNodeBuilder {
2929
&mut self,
3030
_node: &Rc<RuleNode>,
3131
_cursor: &Cursor,
32-
) -> Result<ControlFlow<(), Step>, ()> {
32+
) -> ControlFlow<Result<(), ()>, Step> {
3333
self.stack.push(vec![]);
34-
Ok(ControlFlow::Continue(Step::In))
34+
ControlFlow::Continue(Step::In)
3535
}
3636

37-
fn rule_exit(&mut self, node: &Rc<RuleNode>, cursor: &Cursor) -> Result<ControlFlow<()>, ()> {
37+
fn rule_exit(&mut self, node: &Rc<RuleNode>, cursor: &Cursor) -> ControlFlow<Result<(), ()>> {
3838
let children = self.stack.pop().unwrap();
3939

4040
if (node.kind == RuleKind::LeadingTrivia) | (node.kind == RuleKind::TrailingTrivia) {
4141
if children.is_empty() {
42-
return Ok(ControlFlow::Continue(()));
42+
return ControlFlow::Continue(());
4343
}
4444
}
4545

@@ -50,10 +50,10 @@ impl Visitor<()> for TestNodeBuilder {
5050
};
5151
self.stack.last_mut().unwrap().push(new_node);
5252

53-
Ok(ControlFlow::Continue(()))
53+
ControlFlow::Continue(())
5454
}
5555

56-
fn token(&mut self, node: &Rc<TokenNode>, cursor: &Cursor) -> Result<ControlFlow<()>, ()> {
56+
fn token(&mut self, node: &Rc<TokenNode>, cursor: &Cursor) -> ControlFlow<Result<(), ()>> {
5757
if !Self::is_whitespace(node) {
5858
let kind = if Self::is_comment(node) {
5959
TestNodeKind::Trivia(node.kind)
@@ -69,7 +69,7 @@ impl Visitor<()> for TestNodeBuilder {
6969
self.stack.last_mut().unwrap().push(new_node);
7070
}
7171

72-
Ok(ControlFlow::Continue(()))
72+
ControlFlow::Continue(())
7373
}
7474
}
7575

@@ -89,7 +89,9 @@ impl TestNode {
8989
let mut visitor = TestNodeBuilder {
9090
stack: vec![vec![]],
9191
};
92-
node.cursor().drive_visitor(&mut visitor).unwrap();
92+
if let ControlFlow::Break(result) = node.cursor().drive_visitor(&mut visitor) {
93+
assert!(result.is_ok());
94+
}
9395
return visitor.stack.remove(0).remove(0);
9496
}
9597

0 commit comments

Comments
 (0)