[BUG]: Usage of a conditional return label results in an unexpected null value

[victorr99]

Current Behavior

Running an expression which uses the BlockAsync and Await methods, in combination with an IfThen and Return(...) lets the expression return an unexpected null value.

Expected Behavior

The actual value which should be returned in the IfThen expression is being returned.

Steps To Reproduce

The example below shows the usage of a conditional return. At no place we return a null value, although a null value is returned when the IfThen branch is being taken.

var expected = new object();
var variable = Variable(typeof(object));
var label = Label(typeof(object), "return");

var lambda =
    Lambda<Func<Task<object>>>(
        BlockAsync(
            [variable],
            Assign(
                variable,
                Await(
                    Constant(Task.FromResult(expected)))),
            IfThen(
                NotEqual(
                    variable,
                    Constant(null, typeof(object))),
                Return(
                    label,
                    variable,
                    typeof(object))),
            Return(
                label,
                Constant(
                    new object(),
                    typeof(object)),
                typeof(object)),
            Label(
                label,
                Constant(
                    new object(),
                    typeof(object)))
        ));

var method = lambda.Compile();
var result = await method();

Assert.Same(expected, result); // == false, but true was expected here.

Anything else?

When I modify the expression tree to a non-async variant, I do get the result I'm expecting.
You may verify this with the example below:

var expected = new object();
var variable = Variable(typeof(object));
var label = Label(typeof(object), "return");

var lambda =
    Lambda<Func<object>>(
        Block(
            [variable],
            Assign(
                variable,
                Constant(expected)),
            IfThen(
                NotEqual(
                    variable,
                    Constant(null, typeof(object))),
                Return(
                    label,
                    variable,
                    typeof(object))),
            Return(
                label,
                Constant(
                    new object(),
                    typeof(object)),
                typeof(object)),
            Label(
                label,
                Constant(
                    new object(),
                    typeof(object)))
        ));

var method = lambda.Compile();
var result = method();

Assert.Same(expected, result); // == true in this case

It seems that SetResult on the AsyncStateMachine isn't being called (or not properly).

I'm aware that I can use an IfThenElse to prevent the need of return labels. Our use case however requires a lot of early bail outs which will grow exponentially. Nesting the IfThenElse would result in deep nested IfThenElse expressions (a depth of 30+ isn't uncommon in our case).

Thanks in advance!

[github-actions]

Could not create branch bugfix/123-bug-usage-of-a-conditional-return-label-results-in-an-unexpected-null-value due to: Resource not accessible by integration - https://docs.github.com/rest/git/refs#create-a-reference

[github-actions]

Could not create PR (${e.message})
Context:

{
  "owner": "Stillpoint-Software",
  "repo": "hyperbee.expressions",
  "branchName": "bugfix/123-bug-usage-of-a-conditional-return-label-results-in-an-unexpected-null-value",
  "base": "develop",
  "prTitle": "[BUG]: Usage of a conditional return label results in an unexpected null value",
  "draft": true,
  "username": "bfarmer67",
  "baseHeadSha": "434b1d37b01b807b90814b897279cebe80bc5fbf"
}

[bfarmer67]

@victorr99 exponentially nested IfThenElse would be horrible. I'm looking in to this. Thanks for reporting.

[bfarmer67]

Hi @victorr99,

I wanted to give you an update on progress:

Fixed Issues

I am testing fixes for both issues:

Issue #123 (Return labels not assigning _finalResultVariable):

Identified the Root cause in VariableResolver.VisitGoto - Return gotos inside non-lowered expressions weren't being converted to assign the result variable
Fixed by wrapping Return values in Assign(_finalResultVariable, value) when appropriate

Issue #126 (Missing boxing for value-type assignments):

You were spot on about the missing Convert. Added proper boxing conversion in Transition.SetResult when assigning value types to object-typed variables

FastExpressionCompiler Limitation Discovered

During testing, I discovered an issue with FastExpressionCompiler (FEC) that affects the fixes:

FEC has documented error 1007 (NotSupported_Try_GotoReturnToTheFollowupLabel) for Return gotos from TryCatch blocks. However, the detection is incomplete - it only catches simple patterns like Return(label, variable) but misses compound expressions like Return(label, Assign(...)).

Impact: Instead of returning null to trigger fallback to System compiler, FEC generates invalid IL that throws an InvalidProgramException at runtime. This breaks the documented fallback mechanism.

I've filed: dadhi/FastExpressionCompiler#495

Workaround: Until FEC returns null for unsupported patterns you need to compile with 'System` for the use case you outlined. If FEC puts in a fix for the unsupported patterns, the fallback would work transparently.

Current Status

• Fixes implemented and tested on branch bug-fix-123-126
• Tests passing for System and Interpret compilers for 123
• FEC issue filed and documented

The fixes are ready and work correctly. The FEC limitation is documented and won't block your use case as long as you're using the standard System Compile() mechanism.

Let me know if you have any questions. I would like to hear back from FEC before merging - but he is usually super responsive. I should be able to get this published in the next day or so.

[victorr99]

@bfarmer67 thank you for the status update. I really appreciate the effort you're putting into it to fix the issues. Thanks!

[bfarmer67]

@victorr99 I created a release. Until we have a resolution to the FEC pattern detection for error 1007: NotSupported_Try_GotoReturnToTheFollowupLabel you will have to use System Compile. When there is an update for the pattern detection I will create a new release (hopefully soon).

[victorr99]

@bfarmer67 thanks for the update and the release. I'll check it out soon.