Skip to content

Expect source operators to bind in AOT in the tuple conversion matrix - #7580

Merged
thomhurst merged 1 commit into
mainfrom
issue-7579-aot-tuple-matrix
Nov 4, 2026
Merged

thomhurst merged 1 commit into
mainfrom
issue-7579-aot-tuple-matrix

Conversation

@thomhurst

@thomhurst thomhurst commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

Fixes #7579.

Problem

On main, TupleConversionMatrixTests.Binding fails in AOT mode on every OS, so every PR's AOT checks fail. The matrix (#7559) expected four source-defined operator rows not to bind in AOT, citing #7511. #7547 fixed #7511 by discovering AOT converters from method data source return types, and it merged at 06:04. #7559's last CI run was at 05:37, before that merge, so its expectation was stale when it merged at 06:21. On main, ExactOperator, TupleOperator, NullableTupleOperator and ExplicitOperator now bind in AOT, as they already do in JIT and Reflection mode.

Change

Those four rows no longer set BindsAot: false, and the comment above them now says why they bind. No runtime code changes. The generated ConversionFixtures.cs does not depend on BindsAot, so it is unchanged.

Validation

  • Published TUnit.TestProject with Native AOT (osx-arm64) and ran /*/TUnit.TestProject.TupleConversions/*/*. 58 tests ran, 40 passed and 18 failed, and all 29 rows match the updated BindsAot expectations: the 9 rejected rows fail at binding and the 20 others pass.
  • TUnit.Analyzers.Tests TupleConversionMatrixTests: 30/30 passed.
  • TUnit.Engine.Tests is not run locally (repository policy); CI's AOT and Engine tests checks cover it.

Summary by CodeRabbit

  • Tests
    • Updated coverage for tuple conversion scenarios to use the default expectations for AOT binding.
    • Confirmed that the explicitly rejected operator scenario remains rejected. These changes affect test validation only and do not introduce user-facing functionality.

#7547 (closing #7511) made Native AOT discover converters from method data source
return types, so source-only implicit and explicit operators now bind in AOT. #7559
added the matrix with the earlier expectation and its last CI run predates #7547, so
on main TupleConversionMatrixTests.Binding fails in AOT mode: ExactOperator,
TupleOperator, NullableTupleOperator and ExplicitOperator now bind.

Fixes #7579
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T09:02:13.786956Z 5203c59 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The tuple conversion matrix removes explicit AOT non-binding expectations from three implicit-operator cases and one explicit-operator case. The explicit-operator case retains Accepted: false.

Changes: AOT tuple conversion expectations

Layer / File(s) Summary
Update operator case expectations
tests/SharedTestHelpers/TupleConversionMatrix.cs
Three implicit-operator cases and the explicit-operator case now use the default AOT binding expectation. The explicit-operator case remains unaccepted.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other · Severity of issue fixed: Medium

Possibly related issues

  • TUnit issue 7530 requests a shared matrix for analyzer and runtime tuple-conversion parity, including AOT converter behavior. This change updates AOT expectations for operator cases in that matrix.

Possibly related PRs

  • thomhurst/TUnit#7559: Introduced the shared tuple conversion matrix and the operator rows updated here.
  • thomhurst/TUnit#7547: Changed AOT converter discovery for operators on method-data-source return types, including tuple elements.
Merge Risk: Minimal · up to 5203c

No merge-blocking issue was identified in this test-expectation change.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: updating tuple conversion matrix expectations so source operators bind in AOT.
Linked Issues check Passed The active linked issue is #7579. The diff updates the four affected rows in TupleConversionMatrix.cs so ExactOperator, TupleOperator, NullableTupleOperator, and ExplicitOperator use the AOT…
Out of Scope Changes check Passed The pull request changes only tuple conversion matrix test expectations and the related explanatory comment. These changes directly address #7579. No unrelated runtime or generated fixture changes are…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check

Explanation

The active linked issue is #7579. The diff updates the four affected rows in TupleConversionMatrix.cs so ExactOperator, TupleOperator, NullableTupleOperator, and ExplicitOperator use the AOT binding expectation that matches the current converter discovery behavior. The comment records the reason. The reported Native AOT validation states that all 29 rows match the updated expectations. Closed issue #7511 supplies historical context only.

Full details: Out of Scope Changes check

Explanation

The pull request changes only tuple conversion matrix test expectations and the related explanatory comment. These changes directly address #7579. No unrelated runtime or generated fixture changes are present in the supplied diff.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

A rabbit checks each tuple row,
As operator expectations grow.
Three implicit cases change their tune,
One explicit case stays unaccepted, too.
The AOT matrix marks the way,
Then hops through tests to greet the day.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5f7697c2-e6ca-45df-96b0-67a8df9972fb
Commits

Reviewing files that changed from the base of the PR and between 98b0697 and 5203c59.

Files selected for processing: 1
  • tests/SharedTestHelpers/TupleConversionMatrix.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Review: LGTM. This is a test-expectation-only change in TupleConversionMatrix.cs. It drops the stale BindsAot: false from the four source-operator rows (ExactOperator, TupleOperator, NullableTupleOperator, ExplicitOperator). #7547 fixed #7511, so those rows now bind in AOT, and the updated comment says why. The PR description shows an AOT run and the analyzer matrix tests passing.

One optional note. This matrix drifted once already, and it only surfaced as red AOT checks on every PR. A short comment on the BindsAot flag could name the open bug that justifies each false value, so a stale expectation is easier to spot. No blocking issues.

@greptile-apps

greptile-apps Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge.

Summary

Updates four tuple conversion rows to expect successful AOT binding.

  • AOT tuple tests now expect source-defined operators to bind.

Reviews (1) · Last reviewed commit: "Expect source operators to bind in AOT i..." · Reviewed by Greptile

@thomhurst

Copy link
Copy Markdown
Owner Author

Thanks. I'm leaving the optional note as is: no open bug backs the 9 rows that still set BindsAot: false. Those are conversions AOT rejects by design, such as narrowing, incompatible types, nested tuple widening, composed operators, and native-integer or char-to-floating conversions the generated binder doesn't perform. The existing comment above the matrix already says why runtime expectations differ from analyzer policy. The four rows this PR changes were the only ones tied to a bug (#7511), and that comment now names the fix that made them bind.

@thomhurst
thomhurst merged commit bdd2aa7 into main Nov 4, 2026
29 checks passed
@thomhurst
thomhurst deleted the issue-7579-aot-tuple-matrix branch November 4, 2026 09:41
This was referenced Nov 4, 2026

This branch was previously deployed

1 inactive deployment
Pull Requests — 5203c590 Deployed Nov 4, 2026 by thomhurst via modularpipeline (windows-latest) #21162
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant