Non-reducible extension nodes are not expanded

[avonwyss]

Describe the bug
First some context: I have written a library which allows creating state machine lambdas with continuations similar to what the C# compiler does for async and iterator methods and lambdas. So you start with a "synchronous" expression tree just like you write an async or iterator method, and some extension nodes are used which represent await X and yield return X. These cannot be reduced, since the cannot be translated to a built-in expression representation.

Now when using the ExpressionTreeToString on these, I get an output like this:

.Lambda #Lambda1<System.Func`3[Xunit.Abstractions.ITestOutputHelper,System.Threading.SemaphoreSlim,System.Threading.Tasks.ValueTask]>(
    Xunit.Abstractions.ITestOutputHelper $var1,
    System.Threading.SemaphoreSlim $var2) {
    .Try {
        .Block(System.Threading.SemaphoreSlim $semaphore) {
            $semaphore = $var2;
            .Extension<bsn.AsyncLambdaExpression.Expressions.AwaitExpression>;
            .Try {
                .Call $var1.WriteLine("In Lock")
            } .Finally {
                .Call $semaphore.Release()
            };
            .Default(System.Threading.Tasks.ValueTask)
        }
    } .Catch (System.Exception $ex) {
        .New System.Threading.Tasks.ValueTask(.Call System.Threading.Tasks.Task.FromException($ex))
    }
}

The AwaitExpression here hat a "Body" expression like this:

.Call $semaphore.WaitAsync()

So the bug is: The code writing the extension fails to write out the "Body".

The DebugView in VS outputs the following:

.Extension<bsn.AsyncLambdaExpression.Expressions.AsyncLockExpression> {
    .Block(System.Threading.SemaphoreSlim $semaphore) {
        $semaphore = $var1;
        .Call $semaphore.WaitAsync();
        .Try {
            .Call $var2.WriteLine("In Lock")
        } .Finally {
            .Call $semaphore.Release()
        }
    }
}$var2$var1

Which is also pretty bad, but in a different way (it doesn't mention the await extension).

Note that even though AwaitExpression and YieldReturnExpression cannot be reduced for the reason stated above, they do implement VisitChildren which then does visit the child expressions.

The current implementation of DebugViewWriterVisitor.WriteExtension is like this, which explains the missing traversal of the inner expressions of non-reducible extension methods:

        protected override void WriteExtension(Expression node) {
            Out(
                $".Extension<{node.GetType()}>"
            );
            if (node.CanReduce) {
[...]
            }
        }

I think a combination of both would be best, e.g. always writing out the extension name, but then try to traverse the children.

[zspitz]

This is a bug, but not for the reasons you note. The ToStringWriterVisitor and DebugViewWriterVisitor were only ever meant as a re-implementation of the built-in representations, which allows mappings between each node and the corresponding substring for these representations as well. If either visitor is doing something different from .NET, that's a bug.

But the built-in representations have a number of limitations aside from (ToStringWriterVisitor) looking like C# until it doesn't, or (DebugWriterVisitor) adding bits of formatting which IMO often obscure more than clarify.

Personally, I prefer the TextualNodeTreeVisitor which produces a clear, simple and consistent representation of each node. Also, it uses reflection to visit any property of a type that can be casted to Expression or IEnumerable<Expression> (source); in your case, assuming Body is exposed as a property of Expression or some inheriting type, it would be visited.

[avonwyss]

Thank you @zspitz for your comment. I did try the different representations some time ago (as in years ago), and none really seemed to be more concise yet detailed than the DebugView. That being said, I'll look into the other options including TextualNodeTreeVisitor and have another look at their output.

[zspitz]

Alternatively, you might want to consider inheriting from DebugWriterVisitor and overriding WriteExtension.