Attempt to pass more detailed information about query expressions to the outside world

erik-kallen commented

2012-07-22 14:17:26 -05:00

(Migrated from github.com)

NRefactory can currently resolve query experssions, in the sense that it can determine that from a in arr select a + 1 is a call to Enumerable.Select<int>, but it does not report other interesting semantics, such as
from a in arr let b = a + 1 select a + b is transformed to something like arr.Select(a => new { a, b = a + 1 }).Select(x => x.a + x.b)

This commit is a POC of how an API for this could look. It contains the following changes:

The QueryExpressionLambda class has been made public. It has a property called RangeVariableMap that can be used to determine how to access local variables inside the lambda body.
The parameter types are correct in QueryExpressionLambdas.
The body for a lambda representing a let clause is something like x => new { x, newValue = expr } (it used to be meaningless).

This is by no means complete work, it's just intended as a starting point for a discussion of how the feature could look. It is also not implemented for the entire pattern, but as a test expression, it works for

    public void Main(string[] args)
    {
        var v = from a in args let i = int.Parse(a) let j = i + 1 where i > j select i + j;
    }

(and I also modified the demo program to show the relevant info).

What do you think?

(Btw, thanks for giving me credit as an author yesterday, it's a great honor).

NRefactory can currently resolve query experssions, in the sense that it can determine that `from a in arr select a + 1` is a call to `Enumerable.Select<int>`, but it does not report other interesting semantics, such as `from a in arr let b = a + 1 select a + b` is transformed to something like `arr.Select(a => new { a, b = a + 1 }).Select(x => x.a + x.b)` This commit is a POC of how an API for this could look. It contains the following changes: 1) The QueryExpressionLambda class has been made public. It has a property called `RangeVariableMap` that can be used to determine how to access local variables inside the lambda body. 2) The parameter types are correct in `QueryExpressionLambda`s. 3) The body for a lambda representing a let clause is something like `x => new { x, newValue = expr }` (it used to be meaningless). This is by no means complete work, it's just intended as a starting point for a discussion of how the feature could look. It is also not implemented for the entire pattern, but as a test expression, it works for ``` public void Main(string[] args) { var v = from a in args let i = int.Parse(a) let j = i + 1 where i > j select i + j; } ``` (and I also modified the demo program to show the relevant info). What do you think? (Btw, thanks for giving me credit as an author yesterday, it's a great honor).

erik-kallen commented

2012-07-22 14:35:59 -05:00

(Migrated from github.com)

I would have preferred to make the body of the QueryExpressionLambda contain the actual lambda, but this requires:

that the resolver can use aliases, so it knows that when it sees the variable i, it substitutes something else (which wasn't hard to do, I did it but then I discarded the implementation), and
that the node can be scanned twice, once for storing the result in the tree so the local can be represented as itself when resolving the query clause, and once so it can be represented as a member access chain from a parameter when stored in the lambda body. This was something I didn't even start thinking too much about since I would have no idea where to start (a nested visitor, perhaps).

I would have preferred to make the body of the `QueryExpressionLambda` contain the actual lambda, but this requires: 1) that the resolver can use aliases, so it knows that when it sees the variable `i`, it substitutes something else (which wasn't hard to do, I did it but then I discarded the implementation), and 2) that the node can be scanned twice, once for storing the result in the tree so the local can be represented as itself when resolving the query clause, and once so it can be represented as a member access chain from a parameter when stored in the lambda body. This was something I didn't even start thinking too much about since I would have no idea where to start (a nested visitor, perhaps).

dgrunwald commented

2012-07-23 04:45:01 -05:00

(Migrated from github.com)

I think this is going to get horribly complicated very fast. The current query expression resolve logic already is one of the more complicated portions of the resolver.
I think the current semantic tree is OK - the lambda body will point to the actual range variable. The whole "transparent identifier" mess is only really interesting for compilers - for other analysis tools, it might even make the analysis harder (e.g. analysing how the data is flowing through the whole lambda).

Alternative idea: query expressions are defined as a pure syntactic transformation in the C# specification. We could implement that transformation in NRefactory, and you could just apply it after parsing, prior to resolving.

I think this is going to get horribly complicated very fast. The current query expression resolve logic already is one of the more complicated portions of the resolver. I think the current semantic tree is OK - the lambda body will point to the actual range variable. The whole "transparent identifier" mess is only really interesting for compilers - for other analysis tools, it might even make the analysis harder (e.g. analysing how the data is flowing through the whole lambda). Alternative idea: query expressions are defined as a pure syntactic transformation in the C# specification. We could implement that transformation in NRefactory, and you could just apply it after parsing, prior to resolving.

dgrunwald commented

2012-07-23 04:53:29 -05:00

(Migrated from github.com)

In fact the syntactic transformation could also be done within the ResolveVisitor, using a nested visitor to resolve the transformed AST.
But that causes the same issues as the nested visitor in general: the children of the queries' ResolveResult aren't related to the ResolveResults for the child AST nodes anymore. (the Dictionary<LambdaResolveResult, LambdaExpression> idea from #40).

I think you just can't have both at the same time - either you resolve range variables to a LocalResolveResult as the code currently does, or you resolve them to their underlying representation (lambda parameter, member of anonymous type, ...).
Maybe we could introduce an option on CSharpAstResolver that you can use to determine how you want queries to be resolved?

In fact the syntactic transformation could also be done within the ResolveVisitor, using a nested visitor to resolve the transformed AST. But that causes the same issues as the nested visitor in general: the children of the queries' ResolveResult aren't related to the ResolveResults for the child AST nodes anymore. (the `Dictionary<LambdaResolveResult, LambdaExpression>` idea from #40). I think you just can't have both at the same time - either you resolve range variables to a `LocalResolveResult` as the code currently does, or you resolve them to their underlying representation (lambda parameter, member of anonymous type, ...). Maybe we could introduce an option on `CSharpAstResolver` that you can use to determine how you want queries to be resolved?

dgrunwald commented

2012-07-23 05:02:02 -05:00

(Migrated from github.com)

As for the implementation of the syntactic transformation in the ResolveVisitor:

In class QueryExpression, add a "Expression Translate()" method. This would clone the AST node and perform query expression translation (§7.16.2)
1b. The translate method also somehow needs to provide a mapping between old AST nodes and new AST nodes (Dictionary<AstNode, AstNode>?)
In the ResolveVisitor, when resolving a query expression and the new option is enabled, perform the query expression translation and create a nested visitor for the new AST.
Fully resolve in the nested visitor.
Copy the results back to the parent visitor (similar to LambdaTypeHypothesis.MergeInto), using the Dictionary<AstNode, AstNode> to translate between new AST and original AST.

But it's probably easier just to implement an in-place QueryExpression.Translate() and let the compiler call that.

As for the implementation of the syntactic transformation in the ResolveVisitor: 1. In `class QueryExpression`, add a "Expression Translate()" method. This would clone the AST node and perform query expression translation (§7.16.2) 1b. The translate method also somehow needs to provide a mapping between old AST nodes and new AST nodes (`Dictionary<AstNode, AstNode>`?) 2. In the ResolveVisitor, when resolving a query expression and the new option is enabled, perform the query expression translation and create a nested visitor for the new AST. 3. Fully resolve in the nested visitor. 4. Copy the results back to the parent visitor (similar to `LambdaTypeHypothesis.MergeInto`), using the `Dictionary<AstNode, AstNode>` to translate between new AST and original AST. But it's probably easier just to implement an in-place `QueryExpression.Translate()` and let the compiler call that.

erik-kallen commented

2012-07-23 06:00:35 -05:00

(Migrated from github.com)

My only reason for doing this was that the code that does much of the work that I need is already some 500 lines of complicated code so I thought it would be less work to modify that than to start from scratch. But you're probably right that it is going to be horribly complicated so it will be less work to just implement the syntactic transformation in a separate method.

If this new hypothetical method also returns the mappings, it could probably be used to uncomplicate things in the ResolveVisitor (eg. get rid of the ResolveVisitor.GetTypeForQueryVariable method which won't work when someone does funky stuff).

My only reason for doing this was that the code that does much of the work that I need is already some 500 lines of complicated code so I thought it would be less work to modify that than to start from scratch. But you're probably right that it is going to be horribly complicated so it will be less work to just implement the syntactic transformation in a separate method. If this new hypothetical method also returns the mappings, it could probably be used to uncomplicate things in the ResolveVisitor (eg. get rid of the `ResolveVisitor.GetTypeForQueryVariable` method which won't work when someone does funky stuff).

erik-kallen commented

2012-07-24 10:43:14 -05:00

(Migrated from github.com)

There are better ways to achieve the goal

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

Attempt to pass more detailed information about query expressions to the outside world #82

Pull request closed