Add support of lambda expressions #140
Conversation
metoule
changed the title
|
It looks very promising! Very well! |
…nt, so that it can access the same variables and types. Improve resolution of methods where the parameters are generic types.
…ype (e.g. SelectMany).
… the method's parameters.
…g. It's disabled by default because it has a slight performance cost.
Allow a single lambda expression to be converted to a Func type. Edited documentation.
|
I think the PR is ready for review now. Parsing the lambda expressions has a slight performance penalty, because every statement can be one: we try to parse each statement as a lambda expression, and we backtrack if we don't detect the lambda arrow token. Because of this, I added a new parser settings to enable / disable the lambda expressions parsing. To make all this work, I had to modify a bunch of code related to the generic types; hopefully it won't impact any existing code. |
metoule
changed the title
|
I just released v2.5.0 with all prev PRs... next days I will review in depth this one and we can then have a new release. Thank you again for your recent work! |
|
There appears to be a problem when using nested lambdas with net6.0. Adding net5.0 and net6.0 as unit test targets results in LambdaExpressionTest.Nested_lambda failing only on the net6.0 tests. Here is the test output/exception message: |
|
I think it's linked to the new The issue is not related to nested lambdas: I can reproduce the same issue with this simpler test: [Test]
public void String_single_or_default_lambda()
{
var target = new Interpreter(_options);
var str = "cd";
target.SetVariable("str", "cd");
var result = target.Eval<char>("str.SingleOrDefault(c => c == 'd')");
Assert.AreEqual(str.SingleOrDefault(c => c == 'd'), result);
}It feels like the resolution doesn't find the proper method to use, and therefore can't find the proper type parameters. |
Only promote a lambda expression if the method's parameter is a delegate with the matching number of generic arguments.
|
I managed to find and fix the issue. It occurred when a lambda expression could be matched to a generic argument, for example: public static TSource MySingleOrDefault<TSource>(this IEnumerable<TSource> source, TSource defaultValue)
{
return source.SingleOrDefault();
}
str.MySingleOrDefault(c => c == 'd')In that case, this method should be ignored during resolution, because a lambda expression can't be a generic argument. |
|
If you are able to rebase from master we should have new PR validation using github actions... |
There was a problem hiding this comment.
@metoule When you have some time can you rebase with latest master version?
One small thing: maybe it is better to remove the EnableLambdaExpression function and just leave the possibility to set the feature using InterpreterOptions? What do you think? Just to have a single way to change these kind of options.
Thank you!
|
@davideicardi I've merged the master branch. For |
|
If I can express my opinion, as the options and features increase it is a bit limiting to use an enum. For the future it might be interesting to use a class object that centralize all the options, even the most complex ones. |
The idea for now is to use for all flags (where a simple true/false is enough) just the InterpreterOptions. Where I need more data I used a dedicated function (like |
|
@bortolin |
that makes sense! I've removed the |
There was a problem hiding this comment.
Added some small comments, feel free to discuss it.
But overall I think it is very good! Very good idea and implementation!
Are you planning to do something else? Or do you think that we can then merge it? (in the PR description is still marked as "work in progress" ...)
|
I don't know if it needs to be supported, as I cannot find/think of a usage for it in LINQ, but this implementation does not support parameter-less lambdas e.g. |
|
@Pentiva that was an oversight, thanks for reporting! You can now write a parameterless lambda: [Test]
public void Lambda_expression_no_arguments()
{
var target = new Interpreter(InterpreterOptions.Default | InterpreterOptions.LambdaExpressions);
var lambda = target.Eval<Func<int>>("() => 5 + 6");
Assert.AreEqual(11, lambda.Invoke());
} |
This is a proposition to add support of lambda expressions (to fix #25). This is still a work in progress, but it's a promising start.
In particular, there are a few regressions, and some use cases are not working yet.
I'll keep working on it, but I still wanted to open a PR to have some feedback :)