Skip to content

Fix for RelativeSource AncestorType bindings not generating compiled bindings under AOT#34408

Open
BagavathiPerumal wants to merge 2 commits into
dotnet:net11.0from
BagavathiPerumal:fix-34056
Open

Fix for RelativeSource AncestorType bindings not generating compiled bindings under AOT#34408
BagavathiPerumal wants to merge 2 commits into
dotnet:net11.0from
BagavathiPerumal:fix-34056

Conversation

@BagavathiPerumal

@BagavathiPerumal BagavathiPerumal commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Root Cause:

The issue occurs because KnownMarkups.cs::ProvideValueForBindingExtension had a hasRelativeSource gate that routed all RelativeSource bindings to new Binding(string, ...) — a [RequiresUnreferencedCode] constructor that gets trimmed under AOT Release builds. The gate was necessary to prevent using parent-scope x:DataType as the source type for {RelativeSource Self} bindings. However, it was too broad: when AncestorType={x:Type PageViewModel} is explicitly set, the source type is known at compile time, and a trim-safe TypedBinding should have been generated instead.

Fix Description:

The fix involves restructuring ProvideValueForBindingExtension with a two-step source type resolution:

  • TryGetRelativeSourceAncestorType (new method): looks up the AncestorType's already-resolved ITypeSymbol from the generator's context.Types dictionary. When AncestorType is present and resolvable, it produces a trim-safe TypedBinding<AncestorType, TProperty>.

  • HasExplicitBindingSource (renamed from HasRelativeSourceBinding): guards all other explicit source scenarios — RelativeSource without a resolvable AncestorType (Self, TemplatedParent, FindAncestor without a type) and x:Reference bindings. These bindings fall through to the string-based Binding fallback, since x:DataType does not describe the actual binding source in either case.

This enables compiled bindings precisely where the source type is known at compile time, while keeping the correct fallback for all other RelativeSource modes and x:Reference bindings. Zero new type resolution is needed — it reuses the ITypeSymbol already resolved by the SourceGen pipeline.

Issues Fixed

Fixes #34056

Tested the behaviour in the following platforms

  • iOS
  • Mac
  • Android
  • Windows

Note: NativeAOT scenarios are applicable only to iOS and Mac Catalyst platforms as per the .NET MAUI documentation.

Output Screenshot

Before Issue Fix After Issue Fix
34056-BeforeFix.mov
34056-AfterFix.mov

@github-actions

github-actions Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34408

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34408"

@dotnet-policy-service dotnet-policy-service Bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Mar 10, 2026
@vishnumenon2684 vishnumenon2684 added the community ✨ Community Contribution label Mar 11, 2026
@sheiksyedm sheiksyedm requested a review from Copilot March 11, 2026 05:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a .NET MAUI XAML SourceGen limitation where {RelativeSource AncestorType=...} bindings inside templates were incorrectly routed to string-path Binding(...) creation, which is not trim-safe under AOT. The update enables SourceGen to produce trim-safe TypedBinding when the ancestor type is known at compile time, while preserving the existing fallback behavior for other RelativeSource modes.

Changes:

  • Update KnownMarkups.ProvideValueForBindingExtension to prefer generating a compiled TypedBinding when a RelativeSource has a resolvable AncestorType.
  • Retain the guard that prevents compiling other RelativeSource modes (e.g., Self, TemplatedParent, and untyped ancestor lookups) using ambient x:DataType.
  • Add new XAML unit tests (issue #34056) covering both the fixed ancestor-type scenario and the protected {RelativeSource Self} scenario.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/Controls/src/SourceGen/KnownMarkups.cs Refines SourceGen binding compilation logic to allow compiled bindings for RelativeSource AncestorType when the type is already resolved, avoiding trim-unsafe string-path bindings under AOT.
src/Controls/tests/Xaml.UnitTests/Issues/Maui34056.xaml Adds a minimal repro XAML page covering both RelativeSource AncestorType in a DataTemplate and {RelativeSource Self} in a DataTemplate.
src/Controls/tests/Xaml.UnitTests/Issues/Maui34056.xaml.cs Adds unit tests validating the generated binding types across Runtime/XamlC/SourceGen inflators.

Comment thread src/Controls/tests/Xaml.UnitTests/Issues/Maui34056.xaml.cs
Comment thread src/Controls/tests/Xaml.UnitTests/Issues/Maui34056.xaml.cs Outdated
Comment thread src/Controls/tests/Xaml.UnitTests/Issues/Maui34056.xaml.cs Outdated
@sheiksyedm sheiksyedm marked this pull request as ready for review March 11, 2026 06:15
@sheiksyedm sheiksyedm added the xsg Xaml sourceGen label Mar 11, 2026
@sheiksyedm sheiksyedm added this to the .NET 10 SR6 milestone Mar 11, 2026
@sheiksyedm

Copy link
Copy Markdown
Contributor

/azp run maui-pr-uitests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@sheiksyedm

Copy link
Copy Markdown
Contributor

/azp run maui-pr-uitests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@kubaflo

kubaflo commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

@simonrozsival @StephaneDelcroix could you please review this one?

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please resolve conflicts?

@BagavathiPerumal

Copy link
Copy Markdown
Contributor Author

Could you please resolve conflicts?

@kubaflo, I have resolved the merge conflicts in this PR.

@MauiBot MauiBot added s/agent-approved AI agent recommends approval - PR fix is correct and optimal s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates labels Mar 26, 2026
@kubaflo

kubaflo commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

AI code review for net11.0 target

Verdict: LGTM (non-approval automated review; no human approval implied)

Independent diff-first pass, then reconciled with the PR narrative (#34056).

The change narrows the previously-too-broad hasRelativeSource gate in KnownMarkups.ProvideValueForBindingExtension. Now:

  • RelativeSource with a resolvable AncestorType → uses that already-resolved ITypeSymbol as the source type, emitting a trim-safe TypedBinding<AncestorType, T> instead of falling back to the [RequiresUnreferencedCode] string Binding ctor (the AOT/trim failure).
  • RelativeSource without a resolvable type (Self / TemplatedParent / FindAncestor-without-type) and x:Reference → still fall through to the string Binding, correctly avoiding misuse of the parent-scope x:DataType as the source.

Observations (non-blocking):

  • TryGetRelativeSourceAncestorType relies on context.Types[typeExtNode] already being populated by ProvideValueForRelativeSourceExtension. That ordering assumption is the load-bearing part; if a future change reorders extension processing, AncestorType bindings silently regress to the string path. A short assert/comment guarding the invariant would harden it (the comment already documents the intent — good).
  • Tests are well-targeted: Scenario 1 asserts the compiled TypedBinding<PageVM, ICommand> for XamlC + SourceGen, and Scenario 2 deliberately uses a path that exists on the item VM so the test proves the guard (not a failed lookup) is what blocks compiling {RelativeSource Self}. The XamlC-vs-SourceGen behavioural split is explicitly documented.

CI: required pipelines are red, but the failing legs (AOT macOS, RunOniOS_MauiRelease*, broad Android UI/device test groups) look like the known CoreCLR R2R infra breakage (#34303) and unrelated flake rather than a SourceGen-only change. Recommend confirming the Xaml.UnitTests leg is green before merge.

Confidence: medium-high on correctness of the fix; medium on CI attribution (couldn't fully confirm every red leg is infra).

@jfversluis

Copy link
Copy Markdown
Member

/rebase

@kubaflo

kubaflo commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

AI code review refresh for net11.0 target

Head reviewed: 55b3658

Verdict: Blocked/limited (code remains LGTM from this refresh, but CI is not green)

Findings: No new code-blocking findings. The only new post-review activity I found after the 2026-06-01 marker was the /rebase comment by jfversluis on 2026-06-03; the head SHA is unchanged and the PR is still open against net11.0. I re-inspected the diff and head files: the SourceGen change correctly permits RelativeSource AncestorType={x:Type ...} to use the resolved ancestor symbol for trim-safe TypedBinding, while preserving the fallback for RelativeSource Self/runtime-source cases. The added XAML unit tests cover both the fixed AncestorType path and the Self guard.

Confidence: Medium-high on the code path and tests; limited by CI remaining red/pending.

CI note: gh pr checks currently reports broad failures/pending work (including Build Analysis and many maui-pr-uitests jobs; summary observed: 87 pass, 112 fail, 5 pending, 10 cancelled). These failures do not look specifically tied to this small XAML SourceGen/test diff from the check names alone, but I cannot call the PR ready until required checks are green or classified.

Non-approval disclaimer: this is an automated refresh comment only, not a GitHub approval or request-changes review.

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PR #34408 — Cross-pollination review

Verdict: NEEDS_DISCUSSION (confidence: medium)

3 of 4 models landed on LGTM (gpt-5.5 with 0 findings; gemini-3.1-pro and opus-4.6 with the same single ValueNode suggestion → 2-model consensus); opus-4.8 returned NEEDS_DISCUSSION after flagging a possible new-diagnostic regression. Reading the actual code confirms opus-4.8's concern is real, so it drives the verdict. The fix itself is sound — it correctly narrows the old RelativeSource gate so AncestorType bindings compile to trim-safe TypedBindings while Self / TemplatedParent / x:Reference still fall back to the string Binding.

Key findings

  • (warning) KnownMarkups.cs:368 — possible new MAUIG2045 diagnostic. On the AncestorType path xRefSourceType stays null, so a failed TryCompileBinding now reports BindingPropertyNotFound (a Warning) for bindings that were never compiled before this PR. This is inconsistent with the x:Reference suppression directly below and could break TreatWarningsAsErrors builds. Confirmed in code (CompiledBindingMarkup.cs:319, Descriptors.cs:228); worth a maintainer decision on whether it's intended. Common MVVM paths ([ObservableProperty]/[RelayCommand]) are still covered.
  • (suggestion) KnownMarkups.cs:765 — ElementNode-only AncestorType. 2-model consensus (gemini + opus-4.6). The string/ValueNode form of AncestorType isn't written to context.Types, so it isn't picked up here and falls back to the string Binding. Not a regression — a safe fallback and a missed enhancement only; downgraded accordingly.
  • (suggestion) Maui34056.xaml.cs:57 — test asserts binding type, not resolution. Optional: add an end-to-end assert (content.Command == pageVm.TestCommand) to guard source-type/getter correctness, since the non-Element AncestorType resolves via FindAncestorBindingContext.

CI

Red, but mixed. Framework Build, Pack, and Helix Unit Tests pass (the new XAML tests run), as do AOT windows and RunOniOS_MauiNativeAOT. The bulk of failures are UI/device-test "Run" legs failing uniformly across unrelated control categories and platforms — a classic infra signature, very unlikely caused by a 3-line SourceGen change. However, a few trim/AOT integration legs in this PR's exact domain (AOT macOS, RunOniOS_MauiRelease, RunOniOS_MauiReleaseTrimFull) fail while siblings (NativeAOT, MauiRelease_CoreCLR, AOT windows) pass; these should be confirmed as infra/pre-existing — not a regression from this change — before merge.

if (TryGetRelativeSourceAncestorType(markupNode, context, out var ancestorTypeSymbol)
&& ancestorTypeSymbol is not null)
{
dataTypeSymbol = ancestorTypeSymbol;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When the AncestorType path is taken here, xRefSourceType stays null. If TryCompileBinding then fails because the Path can't be resolved on the AncestorType, the guard a few lines below — if (propertyNotFoundDiagnostic is not null && xRefSourceType is null) — is satisfied and MAUIG2045 BindingPropertyNotFound (a Warning) is reported (see CompiledBindingMarkup.TryParsePath).

RelativeSource AncestorType bindings were never compiled before this PR (the old hasRelativeSource gate routed them straight to the string Binding), so this surfaces a new diagnostic for them — and would break builds using TreatWarningsAsErrors. That's inconsistent with the adjacent x:Reference handling, which deliberately suppresses the same diagnostic for exactly the "never compiled before" reason (see the comment immediately below).

TryGetProperty does infer CommunityToolkit.Mvvm [ObservableProperty]/[RelayCommand] members, so common MVVM paths are covered; but properties produced by other source generators (or genuine typos) on the AncestorType will now warn where they previously didn't. Was emitting the diagnostic on this path intentional? If not, consider tracking that the source type came from the AncestorType (e.g. a local sourceFromAncestorType flag set in this branch) and adding && !sourceFromAncestorType to the suppression condition, mirroring x:Reference.

// The AncestorType is typically an x:Type extension (ElementNode).
// ProvideValueForRelativeSourceExtension already resolved this type
// and registered it in context.Types — just look it up.
if (ancestorTypeNode is ElementNode typeExtNode)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TryGetRelativeSourceAncestorType only resolves the AncestorType when it's an ElementNode (the {x:Type ...} form). ProvideValueForRelativeSourceExtension also accepts the bare string form (AncestorType=local:MyViewModel, a ValueNode — see its handling at ~L190-200), but that path doesn't register the resolved type in context.Types, so this lookup misses it and string-form AncestorType bindings silently fall through to the runtime Binding.

This isn't a regression (they weren't compiled before either) — just a missed enhancement for a rarer syntax. If you choose to support it, add a ValueNode branch that resolves the type inline, parsing with the RelativeSource node's NamespaceResolver (as ProvideValueForRelativeSourceExtension does), not the binding node's. Otherwise a short comment noting the string form is intentionally left to the runtime fallback would help future readers.

{
// Both XamlC and SourceGen produce a trim-safe TypedBinding when x:DataType is present
// on the binding node alongside RelativeSource AncestorType.
Assert.IsType<TypedBinding<Maui34056PageViewModel, ICommand>>(binding);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Optional: this asserts only the binding's runtime kind (TypedBinding vs Binding), which proves trim-safety but not that the compiled binding actually resolves to the right member. Because the AncestorType (Maui34056PageViewModel) isn't an Element, the RelativeSource resolves via FindAncestorBindingContext and the generated getter is handed the ancestor's BindingContext as its source. Consider also exercising it end-to-end — parent the template content under the page and assert content.Command == pageVm.TestCommand — so a wrong source-type/getter would be caught, not just a wrong binding type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-fix-win AI found a better alternative fix than the PR s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) xsg Xaml sourceGen

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS wrong trimmed Relative bindings under config XamlInflator and AOT