Skip to content

Add sorting and comparison regression tests as dedicated Issue499 file - #500

Merged
dadhi merged 6 commits into
masterfrom
copilot/fix-invalidprogramexception-sorting
Apr 8, 2026
Merged

Add sorting and comparison regression tests as dedicated Issue499 file#500
dadhi merged 6 commits into
masterfrom
copilot/fix-invalidprogramexception-sorting

Conversation

Copilot AI commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Issue #499 reports InvalidProgramException at runtime (not compile time) for expression trees representing a quicksort partition and a comparison function when compiled with CompileFast.

New file: Issue499_InvalidProgramException_for_Sorting_and_comparison_function

Two minimal, self-contained test methods that reproduce the patterns from the original SortCompiler report without requiring SortData/ColumnData infrastructure:

Quicksort_partition_with_nested_loops

Mirrors SortCompiler.Compile — an outer void Loop containing two inner Loops using the makeCondition pattern: conditional Continue(continueN) + fallthrough Break(endSubN).

Loop(
    Block(
        IfThen(GreaterThan(i, j), Break(endMain)),
        Loop(                                           // while arr[i] < pivot: i++
            Block(
                Assign(compareResult, Call(arr[i], compareTo, pivot)),
                IfThen(Equal(compareResult, Constant(-1)),
                    Block(PostIncrementAssign(i), Continue(continue1))),
                Break(endSub1)),
            endSub1, continue1),
        Loop(/* symmetric for j-- */),
        IfThen(LessThanOrEqual(i, j), /* swap */)),
    endMain)

Comparison_function_with_goto_labels

Mirrors SortCompiler.CompileComparisonFunctionReturn(label, value) (GotoExpression with Kind=Return) inside a conditional, followed by a fallthrough Return and a typed Label default.

Block(
    new[] { compareResult },
    Assign(compareResult, Call(left, compareTo, right)),
    Assign(compareResult, Negate(compareResult)),          // DESC: invert
    IfThen(Equal(compareResult, Constant(-1)), Return(endMain, Constant(-1))),
    Return(endMain, Constant(1)),
    Label(endMain, Constant(-1)))

Each test runs the expression with CompileSys to get a reference result, then asserts CompileFast produces the same output. The tests passing confirms the fix from issue #498 covers these patterns.

TestRunner registration

The Issue499 struct is registered in both TestRunner projects:

  • TestRunner/Program.cs (net6/net8/net9) — via st.Run() and lt.Run() in Main()
  • TestRunner.Net472/Program.cs — via fecTests.Run() and the LightExpression thread in RunTestsX()

Issue498 is also now registered in TestRunner.Net472/Program.cs for both FEC (fecTests.Run) and LightExpression (background thread), ensuring it runs on .NET 4.7.2. Additionally, other previously missing ITestX tests were added to the Net472 runner: Issue398, Issue476, Issue480, and Issue495.

Copilot AI changed the title [WIP] Fix InvalidProgramException for sorting and comparison function Add sorting and comparison regression tests to Issue498 Apr 7, 2026
Copilot AI requested a review from dadhi April 7, 2026 16:15
Copilot finished work on behalf of dadhi April 7, 2026 16:15
@dadhi

dadhi commented Apr 7, 2026

Copy link
Copy Markdown
Owner

@copilot Please move the tests to the separate Issue499_... file. Use 488 just for reference.
Also if tests are passing it means there is no issue anymore right?

@dadhi
dadhi marked this pull request as ready for review April 7, 2026 20:33

@dadhi dadhi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aldo if the tests can be run in Net472 please add them to both .net 9 and .net472 TestRunner projects.

{
Original_test(t);
Quicksort_partition_with_nested_loops(t);
Comparison_function_with_goto_labels(t);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the tests to the separate Issue499_... file. I mentioned Issue488 just for reference.
Each issue ideally should have a separate/dedicated test file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in e428c53. Tests are now in the dedicated Issue499_InvalidProgramException_for_Sorting_and_comparison_function.cs file, and Issue498 is back to its original state.

Copilot AI commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

Aldo if the tests can be run in Net472 please add them to both .net 9 and .net472 TestRunner projects.

Done in e428c53. The Issue499 tests are added to both TestRunner/Program.cs (net6/net8/net9) and TestRunner.Net472/Program.cs via RunTestsX() for both FEC and LightExpression runners.

@copilot Please move the tests to the separate Issue499_... file. Use 488 just for reference.
Also if tests are passing it means there is no issue anymore right?

Tests are now in a dedicated Issue499_InvalidProgramException_for_Sorting_and_comparison_function.cs file (commit e428c53). Regarding whether the issue is fixed: the tests pass with CompileFast, which confirms the fix from issue #498 already covers these patterns. If they were failing before that fix, these tests serve as regression coverage.

Copilot AI changed the title Add sorting and comparison regression tests to Issue498 Add sorting and comparison regression tests as dedicated Issue499 file Apr 7, 2026
Copilot finished work on behalf of dadhi April 7, 2026 20:47
Copilot AI requested a review from dadhi April 7, 2026 20:47

@dadhi dadhi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let ensure we running the 498 on .net472 as well. Both for Fec and LightExpression
Compare the rest of TestX implementing tests, may be we forget to run something else.

Copilot AI commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

Let ensure we running the 498 on .net472 as well. Both for Fec and LightExpression
Compare the rest of TestX implementing tests, may be we forget to run something else.

Done in 45ff92c. Added Issue498 to TestRunner.Net472 for both FEC (fecTests.Run) and LightExpression (the background thread). Also added the other missing ITestX tests: Issue398, Issue476, Issue480, and Issue495.

Copilot AI requested a review from dadhi April 7, 2026 21:08
Copilot finished work on behalf of dadhi April 7, 2026 21:08

@dadhi dadhi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows workflow issue - may be exposing some error. Please fix:

Run dotnet run --no-build -c:Release --project test/FastExpressionCompiler.TestsRunner.Net472

### TestX runs on FEC tests (UnitTests and IssueTests) and FEC.LightExpression tests in ||
FEC tests are passing in 262 ms.
Unexpected exception in test 'Issue498_InvalidProgramException_when_using_loop':
'System.ArgumentException: Expression of type 'System.Void' cannot be used for return type 'System.Int32'
   at System.Linq.Expressions.Expression.ValidateLambdaArgs(Type delegateType, Expression& body, ReadOnlyCollection`1 parameters)
   at System.Linq.Expressions.Expression.Lambda[TDelegate](Expression body, String name, Boolean tailCall, IEnumerable`1 parameters)
   at System.Linq.Expressions.Expression.Lambda[TDelegate](Expression body, Boolean tailCall, IEnumerable`1 parameters)
   at FastExpressionCompiler.LightExpression.Expression`1.ToLambdaExpression() in /_/src/FastExpressionCompiler.LightExpression/Expression.cs:line 5707
   at FastExpressionCompiler.LightExpression.IssueTests.Issue498_InvalidProgramException_when_using_loop.Original_test(TestContext t) in D:\a\FastExpressionCompiler\FastExpressionCompiler\test\FastExpressionCompiler.IssueTests\Issue498_InvalidProgramException_when_using_loop.cs:line 51
   at FastExpressionCompiler.LightExpression.TestRun.Run[T](T test, TestFlags flags) in /_/src/FastExpressionCompiler/TestTools.cs:line 1062'
Just LightExpression tests are passing in 398 ms.
LightExpression tests + Thread New, Start, Join: 402 ms.
--> waited for the LightExpression tests to finish after FEC tests complete for 139 ms.

ERROR: Some tests are FAILED!

…tching System.Linq.Expressions behavior

Agent-Logs-Url: https://github.com/dadhi/FastExpressionCompiler/sessions/a43bd802-b944-48bc-8c8c-c0abc89bc6e6

Co-authored-by: dadhi <39516+dadhi@users.noreply.github.com>

Copilot AI commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Windows workflow issue - may be exposing some error. Please fix:

Run dotnet run --no-build -c:Release --project test/FastExpressionCompiler.TestsRunner.Net472...

Fixed in d6c559a. The root cause was LoopExpression.Type in LightExpression always returning typeof(void), even when a non-void break label was set. This caused ToLambdaExpression() to produce a void-typed block, which failed when wrapping in Lambda<Func<int>>. Changed to BreakLabel?.Type ?? typeof(void) to match System.Linq.Expressions.LoopExpression behavior. All 1655 tests pass on net9.0.

Copilot finished work on behalf of dadhi April 8, 2026 08:30
Copilot AI requested a review from dadhi April 8, 2026 08:30
@dadhi
dadhi merged commit 344fa12 into master Apr 8, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.InvalidProgramException for Sorting and comparison function

2 participants