Skip to content
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

Chained if_then_else application change #8671

Merged
merged 9 commits into from
Feb 22, 2024
2 changes: 1 addition & 1 deletion lib/rust/parser/src/macros/built_in.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ fn export_body<'s>(
/// If-then-else macro definition.
pub fn if_then_else<'s>() -> Definition<'s> {
crate::macro_definition! {
("if", everything(), "then", everything(), "else", everything()) if_body}
("if", everything(), "then", everything(), "else", or(block(), many(not_block()))) if_body}
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not completely sure what is supposed to be a difference between everything() and or(x,not(x)) - in binary logic x or not x is always true - e.g. close to everything(). Here we have or(block(), many(not_block()))) which is almost or(block(), not_block())) - e.g. x or not x.

Copy link
Contributor

Choose a reason for hiding this comment

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

The many is important! The difference is that this will either match a block at the beginning, or a sequence of non-block items--but unlike everything, it won't match a sequence of non-block items followed by a block.

}

/// If-then macro definition.
Expand Down
1 change: 1 addition & 0 deletions lib/rust/parser/src/macros/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ impl<'s> Match<'s> {
| Self::Nothing
| Self::Identifier(_)
| Self::Expected(_, _)
| Self::Block(_)
| Self::NotBlock(_) => {}
Self::Or(box OrMatch::First(item) | box OrMatch::Second(item)) =>
item.build_var_map(tree, validator),
Expand Down
33 changes: 33 additions & 0 deletions lib/rust/parser/src/macros/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ pub enum PatternData {
Expected(String, Pattern),
/// Named pattern. Mainly used for splicing the code in the macro definition body.
Named(String, Pattern),
/// Consume a block.
Block,
/// Consume any item except a block.
NotBlock,
}

/// Constructor.
Expand Down Expand Up @@ -103,6 +107,16 @@ pub fn named(message: impl Into<String>, item: Pattern) -> Pattern {
Pattern::new(PatternData::Named(message.into(), item), matches_empty_input)
}

/// Matches a block.
pub fn block() -> Pattern {
Pattern::new(PatternData::Block, false)
}

/// Matches anything except a block.
pub fn not_block() -> Pattern {
Pattern::new(PatternData::NotBlock, false)
}

impl Pattern {
/// Repeat the current pattern multiple times.
pub fn many(self) -> Self {
Expand Down Expand Up @@ -171,6 +185,7 @@ pub enum Match<'s> {
Identifier(syntax::Item<'s>),
Expected(String, Box<Match<'s>>),
Named(String, Box<Match<'s>>),
Block(Vec<syntax::item::Line<'s>>),
NotBlock(syntax::Item<'s>),
}

Expand Down Expand Up @@ -208,6 +223,7 @@ impl<'s> Match<'s> {
match self {
Self::Nothing => (),
Self::Identifier(item) | Self::NotBlock(item) => out.push(item),
Self::Block(lines) => out.push(syntax::Item::Block(lines)),
Self::Everything(tokens) => out.extend(tokens),
Self::Seq(fst, snd) => {
fst.get_tokens(out);
Expand Down Expand Up @@ -341,6 +357,23 @@ impl Pattern {
Err(input)
},
},
PatternData::Block => match input.pop_front() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like we have some parser combinators, here! @hubertp is currrently trying to get rid of one such library as part of #9089...

None => Err(default()),
Some(syntax::Item::Block(lines)) =>
Ok(MatchResult::new(Match::Block(lines), input)),
Some(t) => {
input.push_front(t);
Err(input)
}
},
PatternData::NotBlock => match input.pop_front() {
None => Err(default()),
Some(t @ syntax::Item::Block(_)) => {
input.push_front(t);
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
Err(input)
}
Some(t) => Ok(MatchResult::new(Match::NotBlock(t), input)),
},
}
}
}
37 changes: 14 additions & 23 deletions lib/rust/parser/src/macros/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,18 +123,17 @@ impl<'a> SegmentMap<'a> {
/// to learn more about the macro resolution steps.
#[derive(Debug)]
pub struct Resolver<'s> {
blocks: Vec<Block>,
blocks: Vec<Block>,
/// The lines of all currently-open blocks. This is partitioned by `blocks`.
lines: Vec<syntax::item::Line<'s>>,
lines: Vec<syntax::item::Line<'s>>,
/// All currently-open macros. These are partitioned into scopes by `blocks`.
macros: Vec<PartiallyMatchedMacro<'s>>,
macros: Vec<PartiallyMatchedMacro<'s>>,
/// Segments of all currently-open macros. These are partitioned by `macros`.
segments: Vec<MatchedSegment<'s>>,
segments: Vec<MatchedSegment<'s>>,
/// Items of all segments of all currently-open macros. These are partitioned by `segments`.
items: Vec<syntax::Item<'s>>,
context: Context,
precedence: syntax::operator::Precedence<'s>,
start_segment: bool,
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like the start_segment idea wasn't the right way to solve the problem. 287e307 (with its Block and NotBlock patterns) is.

items: Vec<syntax::Item<'s>>,
context: Context,
precedence: syntax::operator::Precedence<'s>,
}


Expand All @@ -144,14 +143,13 @@ impl<'s> Resolver<'s> {
/// Create a new resolver, in statement context.
pub fn new_statement() -> Self {
Self {
context: Context::Statement,
precedence: syntax::operator::Precedence::new(),
blocks: default(),
lines: default(),
macros: default(),
segments: default(),
items: default(),
start_segment: false,
context: Context::Statement,
precedence: syntax::operator::Precedence::new(),
blocks: default(),
lines: default(),
macros: default(),
segments: default(),
items: default(),
}
}

Expand Down Expand Up @@ -229,8 +227,6 @@ impl<'s> Resolver<'s> {

/// Append a token to the state.
fn push(&mut self, root_macro_map: &MacroMap, token: Token<'s>) {
let was_start_segment = self.start_segment;
self.start_segment = false;
match token.variant {
token::Variant::Newline(newline) => {
if !self.lines.is_empty() {
Expand All @@ -241,10 +237,6 @@ impl<'s> Resolver<'s> {
self.context = Context::Statement;
}
token::Variant::BlockStart(_) => {
if !was_start_segment {
trace!("Finishing line!!!! {:?}", token);
self.finish_current_line();
}
let macros_start = self.macros.len();
let outputs_start = self.lines.len();
let items = self.items.len();
Expand Down Expand Up @@ -274,7 +266,6 @@ impl<'s> Resolver<'s> {
let items_start = self.items.len();
self.segments.push(MatchedSegment { header, items_start });
self.context = Context::Expression;
self.start_segment = true;
break;
}
Step::NormalToken(item) => {
Expand Down
Loading