support expressions

[rcoh]

eg. sum(response_ms*2). Expressions can be used either inline or as an assignment: let response_ms = response_ms * 2

[rcoh]

where is supported as well as some basic expressions

[rcoh]

equality expressions are supported. Arithmetic needed

[tstack]

So, I'm taking a whack at this and running into an issue with eval_borrowed(): https://github.com/rcoh/angle-grinder/blob/7b356efa1c8beb653808ad85a129c0fbff7f6d49/src/operator.rs#L57

Since the lifetime of the result is the same as the input HashMap, I don't think there's a way to construct an intermediate value for the result of say an addition. Things have worked so far with bool since the true/false values are &'static. I thought about storing the intermediate in the input HashMap, but that is not mutable at this point. What are your thoughts @rcoh ? Should the result be cloned instead of returned by ref? Or, should the input HashMap be made mutable? Or...

[rcoh]

After a brief skim (so this could be totally wrong and I don't remember exactly why EvalBorrowed exists), can you just implement Evaluatable instead of EvalutatbleBorrowed?

On Tue, Jul 21, 2020 at 10:55 PM Tim Stack [email protected] wrote:

So, I'm taking a whack at this and running into an issue with eval_borrowed(): https://github.com/rcoh/angle-grinder/blob/7b356efa1c8beb653808ad85a129c0fbff7f6d49/src/operator.rs#L57

Since the lifetime of the result is the same as the input HashMap, I don't think there's a way to construct an intermediate value for the result of say an addition. Things have worked so far with bool since the true/false values are &'static. I thought about storing the intermediate in the input HashMap, but that is not mutable at this point. What are your thoughts @rcoh https://github.com/rcoh ? Should the result be cloned instead of returned by ref? Or, should the input HashMap be made mutable? Or...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rcoh/angle-grinder/issues/13#issuecomment-662212145, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYKZ2GS3MGRK5M67GMFQTR4ZIITANCNFSM4EYHK5UQ .

[tstack]

can you just implement Evaluatable instead of EvalutatbleBorrowed?

Hmm, I don't think so... most things seem to call eval_borrowed() and it has a big match for the Expr enum that needs to have all cases satisfied. I don't know why EvaluatableBorrowed exists either. Probably for the sake of performance?

[rcoh]

Yeah I was probably trying to avoid cloning. It could really mess things up but we could:

• Switch to cloning during eval (RIP eval borrowed)
• Switch to Rc/Arc for the output of eval to avoid a full clone
• Implement Copy for Data::Value either by cloning the string or by sticking the string in an Rc.

[rcoh]

OK, I played around with it a little bit. It's a midsize refactor, but I think the answer is to replace eval_borrowed with eval_cow:

pub trait EvaluatableCow<T: Clone> {
    fn eval_cow<'a>(&self, record: &'a Data) -> Result<Cow<'a, T>, EvalError>;
}

Cow will allow eval to return something either borrowed or owned and it's pretty ergonomic to use since it implements the std::borrow magic.

I think it may be a pretty big refactoring, so I can take it on if you want.

[tstack]

I removed borrowed and have it working. I'll try to implement the Cow version.

... but, there's a lifetime in the Cow type signature. Won't that still cause the same problem?

[rcoh]

yeah either way -- if it doesn't make much of a difference in the benchmarks just removing borrowed is totally fine.

On Wed, Jul 22, 2020 at 5:26 PM Tim Stack [email protected] wrote:

I removed borrowed and have it working. I'll try to implement the Cow version.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rcoh/angle-grinder/issues/13#issuecomment-662705662, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYKZ67CZNFYEBGHYJYKCLR45KQ3ANCNFSM4EYHK5UQ .

[tstack]

Do you happen to remember what is left to do here?