Introduce Collector variant of Reducer to accomodate the unimplemented list reducer - #436
Conversation
| #[derive(Debug, Clone, Eq, PartialEq)] | ||
| pub struct Collect { | ||
| pub span: Option<Span>, | ||
| pub reduce_operator: token::ReduceOperator, | ||
| pub variable: Variable, | ||
| } |
There was a problem hiding this comment.
Structurally identical to Stat.
I could just rename Stat, but Stat currently returns an optional raw value. List could be a list of other kinds of concepts, so this may be justified.
| pub enum Reducer { | ||
| Count(Count), | ||
| Stat(Stat), | ||
| Collect(Collect), |
There was a problem hiding this comment.
Do we see other collectors existing apart from plain list? (list_unique? list_sorted? ) or should I introduce a flat Reduce::List instead of a Reduce::Collect
There was a problem hiding this comment.
We still need the span and the variable we're collecting, and it's nicer to have a separate struct we can handle rather than an enum variant. It shouldn't be flattened.
As for name, shrug. Though technically first and last could be in Collect as well.
I'd say calling it List is more likely to lead to cross-repo renames, so I'd settle on Collect at least for now.
| // TODO vvvv rename | ||
| Reducer::Stat(Stat::new(span, ReduceOperator::List, visit_var(children.consume_expected(Rule::var)))) | ||
| Reducer::Collect(Collect::new(span, ReduceOperator::List, visit_var(children.consume_expected(Rule::var)))) |
| pub enum Reducer { | ||
| Count(Count), | ||
| Stat(Stat), | ||
| Collect(Collect), |
There was a problem hiding this comment.
We still need the span and the variable we're collecting, and it's nicer to have a separate struct we can handle rather than an enum variant. It shouldn't be flattened.
As for name, shrug. Though technically first and last could be in Collect as well.
I'd say calling it List is more likely to lead to cross-repo renames, so I'd settle on Collect at least for now.
| value::StringLiteral, | ||
| TypeRef, TypeRefAny, | ||
| }; | ||
| use crate::token::ReduceOperatorCollect; |
| Rule::COUNT => Reducer::Count(Count::new(span, children.try_consume_expected(Rule::var).map(visit_var))), | ||
| Rule::MAX => { | ||
| Reducer::Stat(Stat::new(span, ReduceOperator::Max, visit_var(children.consume_expected(Rule::var)))) | ||
| Rule::reducer_stat => { | ||
| let operator = visit_reducer_stat(keyword); | ||
| Reducer::Stat(Stat::new(span, operator, visit_var(children.consume_expected(Rule::var)))) | ||
| } | ||
| Rule::MIN => { | ||
| Reducer::Stat(Stat::new(span, ReduceOperator::Min, visit_var(children.consume_expected(Rule::var)))) | ||
| } | ||
| Rule::MEAN => { | ||
| Reducer::Stat(Stat::new(span, ReduceOperator::Mean, visit_var(children.consume_expected(Rule::var)))) | ||
| } | ||
| Rule::MEDIAN => { | ||
| Reducer::Stat(Stat::new(span, ReduceOperator::Median, visit_var(children.consume_expected(Rule::var)))) | ||
| } | ||
| Rule::STD => { | ||
| Reducer::Stat(Stat::new(span, ReduceOperator::Std, visit_var(children.consume_expected(Rule::var)))) | ||
| } | ||
| Rule::SUM => { | ||
| Reducer::Stat(Stat::new(span, ReduceOperator::Sum, visit_var(children.consume_expected(Rule::var)))) | ||
| } | ||
| Rule::LIST => { | ||
| // TODO vvvv rename | ||
| Reducer::Stat(Stat::new(span, ReduceOperator::List, visit_var(children.consume_expected(Rule::var)))) | ||
| Rule::reducer_collect => { | ||
| let operator = visit_reducer_collect(keyword); | ||
| Reducer::Collect(Collect::new(span, operator, visit_var(children.consume_expected(Rule::var)))) | ||
| } |
2 participants
Product change and motivation
It was placed under Stat, which it is not and caused confusion & crashes in core.