Implement GATs, step 2 #119
Conversation
| self.iter() | ||
| .flat_map(|wc| match wc.lower(env) { | ||
| Ok(v) => v.into_iter().map(Ok).collect(), | ||
| Err(e) => vec![Err(e)], |
There was a problem hiding this comment.
mmh can't it be just => Err(e)? (I know there is convertibility from Vec<Result<T, E>> to Result<Vec<T, E>> but it feels simpler this way in that case)
There was a problem hiding this comment.
Yeah there's a problem with match arms having different types. If I used v.into_iter().map(Ok) then I'd have a Map object which of course wouldn't work. So the least common denominator is Vec. I may be missing something though.
There was a problem hiding this comment.
Ah right sorry, ok this is fine the way you wrote I guess.
There was a problem hiding this comment.
I added ApplyResult to make it more ergonomic at least.
src/ir/lowering/mod.rs
Outdated
| let g: ir::DomainGoal = self.lower(env)?; | ||
| g.cast() | ||
| let goals: Vec<ir::DomainGoal> = self.lower(env)?; | ||
| goals.into_iter().map(|g| g.cast()).collect() |
There was a problem hiding this comment.
You can use the wonderful Caster iterator adaptor I wrote:
goals.into_iter().casted().collect()=)
src/ir/lowering/mod.rs
Outdated
| let consequence: ir::DomainGoal = self.consequence.lower(env)?; | ||
| let consequences: Vec<ir::DomainGoal> = self.consequence.lower(env)?; | ||
| assert_eq!(1, consequences.len()); | ||
| let consequence = consequences.into_iter().next().unwrap(); |
There was a problem hiding this comment.
Ah I understand the problem. So this may not be the right way to do it, because at one point you might want to lower an "AST"-rule which would look like:
T: Iterator<Item = i32> :- blabla
but the problem is that the consequence part (left part) would be translated to: ProjectionEq(<T as Iterator>::Item = i32), T: Iterator so your assertion would fail.
Instead, we would need to output a vector of ProgramClause (again), and we would generate two clauses in my example:
ProjectionEq(<T as Iterator>::Item = i32) :- blabla
T: Iterator :- blabla
I think this is the right way, you might want to get @nikomatsakis's confirmation though.
There was a problem hiding this comment.
Good point. For Clause this is pretty easy.
For Goal it's.. a bit more involved. Bubbling up multiple leaf goals in LowerGoal would complicate the code quite a bit. I think I could handle it with a Goal::And at the lowest level, but I'm not certain if that's sound.
There was a problem hiding this comment.
Meh. This "custom clause" stuff is mostly useful for writing unit tests of the SLG engine etc. I think it's fine to assert things here, though it'd be even nicer to move it to the grammar, so that a Clause always has a limited set of "domain goals" as its "consequence".
| .collect(); | ||
| Ok(Box::new(ir::Goal::Implies(where_clauses?, g.lower(env)?))) | ||
| } | ||
| Goal::And(g1, g2) => { | ||
| Ok(Box::new(ir::Goal::And(g1.lower(env)?, g2.lower(env)?))) | ||
| } | ||
| Goal::Not(g) => Ok(Box::new(ir::Goal::Not(g.lower(env)?))), | ||
| Goal::Leaf(wc) => Ok(Box::new(ir::Goal::Leaf(wc.lower(env)?))), | ||
| Goal::Leaf(wc) => { | ||
| // A where clause can lower to multiple leaf goals; wrap these in Goal::And. |
There was a problem hiding this comment.
Not certain about this yet
There was a problem hiding this comment.
Seems good to me! Also you can use itertools::Itertools and write something like:
leaves.fold1(|goal, leaf| {
ir::Goal::And(Box::new(goal), Box::new(ir::Goal::Leaf(leaf)))
}).expect("at least one goal");(see for example here)
There was a problem hiding this comment.
Bah, itertools should have called this method reduce. =)
|
@tmandry See my comment about |
This refactors LowerWhereClause to permit multiple domain goals from a single AST where clause.
For #116.
For AST Goals and Clauses, I added some assertions that there was only one domain goal, to keep the one-to-many relationship from "leaking" into those traits. I think this is correct, but not too pretty.