Compiled binding path for subclasses - support for mixed collection

[DanielTrommel]

@w-ahmad , back again with an fix/improvement on the Func/BindingPath code.

Ref this PR, #178, in which we worked on generating a Func at runtime once, instead of repeated reflection to get a cell's value.
In the comments on that PR (#178 (comment)), I already mentioned that the current Func is built just using the first instance for which we happen do a GetPropertyValue(), which might cause trouble.

The specific problem: If the instances used for the rows in the table is a mixture of subclasses, and the (first) binding (step) is to an element on the shared parent class, the current code will throw an InvalidCastException when sorting.

The reason is that the first instance that is used to built the Func, dictates the explicit type cast that is done;
An example: suppose the type of the instance is MySubClass and for a column our BindingPath is "MyProperty", the current code in main generates this:

.Lambda #Lambda1<System.Func`2[System.Object,System.Object]>(System.Object $obj) {
    .If ((MySubClass)$obj != null) {
        ((MySubClass)$obj).MyProperty
    } .Else {
        null
    }
}

However, if MyProperty is actually defined on MySuperClass, that code should use a cast to that type instead:

.Lambda #Lambda1<System.Func`2[System.Object,System.Object]>(System.Object $obj) {
    .If ((MySuperClass)$obj != null) {
        ((MySuperClass)$obj).MyProperty
    } .Else {
        null
    }
}

This way, if the CollectionView contains a mixture of instance of different subclasses for MySuperClass, we won't get an InvalidCastException.

Furthermore:

• The above example is only about the first segment in the Binding Path, but the subclassing can affect any segment of the binding path (different subclass instances, causing a potential InvalidCastException), but this PR handles this as well, and I added Tests for the case this subclassing happens further on in the BindingPath
• While merging the latest main, I noticed you added a func for the Setter variant, so I also hardened your code so it will handle subclassing
• Did some small renaming "PropertyPath" -> "BindingPath" (leftover from my initial code, which was not representative)
• Then, your new Setter flow seemed to partially duplicate the Getter flow, and I figured that both flows are basically the same, except for the last segment of the BindingPath: For the Setter func, you do the set action. Hence, I refactored this, which resulted in a bit less code, and should make the difference between the two flows clearer.
  • This refactor gave rise to a subtle bug in the code (thanks to your test case!) which surfaced due to the above, different casting (before, the cast to the more specific subclass would `cover' this). See the last commit for the fix, and the commit description for more details.

The above points are in order and in separate commits.

[Copilot]

Pull request overview

This pull request hardens the compiled binding-path getter/setter generation so it remains safe when evaluating rows from mixed collections (e.g., sibling subclasses), avoiding invalid runtime casts while keeping the fast compiled-expression approach.

Changes:

• Updates binding-path expression-tree generation to cast to the declaring type of each segment (instead of the first encountered runtime subtype) to support mixed sibling row types.
• Refactors setter generation to reuse the shared path-navigation logic used by the getter (only the final segment differs).
• Adds regression tests covering mixed sibling rows and nested subclass scenarios.

Reviewed changes

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

File	Description
src/Extensions/ObjectExtensions.cs	Adjusts compiled getter/setter expression generation to be mixed-collection-friendly and refactors shared navigation logic.
tests/ObjectExtensionsTests.cs	Adds new test coverage for mixed sibling row scenarios across property and indexer paths.

Comments suppressed due to low confidence (1)

src/Extensions/ObjectExtensions.cs:526

• Null-checking current against null will throw during expression construction when current is a non-nullable value type (struct) and the next segment returns a nullable or reference type. That makes binding paths like MyStruct.RefProp fail to compile even though structs cannot be null. Skip the null-check (and the conditional) when the container expression is a non-nullable value type.

            if (nextPropertyAccess.Type.IsValueType && !nextPropertyAccess.Type.IsNullableType())
            {
                // Value types cannot be null, so don't need to check for null, and we can directly assign the property access
                current = nextPropertyAccess;
            }

[DanielTrommel]

@w-ahmad : that suppressed comment does not make sense

[w-ahmad]

LGTM! Thanks for the contribution @DanielTrommel

[DanielTrommel]

@w-ahmad : just noticed that the file contains some warnings; will look into those, and push those minor changes. If you already merge this, I will create a small PR for that

[w-ahmad]

@w-ahmad : just noticed that the file contains some warnings; will look into those, and push those minor changes. If you already merge this, I will create a small PR for that

If you mean the IL warnings, I don’t think we can omit them. Anyway, just let me know if you want me to leave this PR open.

[DanielTrommel]

@w-ahmad: just pushed some minor changes - old naming, and some small comment improvements wrt to old naming.

About the Warnings: We can only suppress the warnings about trimming; not sure it common to suppress those in these cases? There are quite a few, and obscure other potential warnings.

Also: these warnings are about trimming, and since we are runtime compiling here, this code isnt AOT compatible anyhow. You'll need code generator(s) anyways (which also suggested here: #140 (comment)), I think you already worked on that.

So, I think we can safely suppress these warnings?

If wished, at class level we could add this:

// IL2075/IL2070: All Type values flowing through this class originate from concrete runtime instances — via GetType() on live objects, Expression.Type on expression nodes, or as
// elements of GetInterfaces() results. Reflected members (properties, interfaces, indexers) are resolved at expression-tree build time and are guaranteed to be present; the trimmer
// cannot statically verify this because none of these Type references carry
// [DynamicallyAccessedMembers] annotations.
[UnconditionalSuppressMessage("Trimming", "IL2075",
    Justification = "Type values come from concrete runtime instances (GetType(), Expression.Type, GetInterfaces()). Reflected members are present at expression-tree build time.")]
[UnconditionalSuppressMessage("Trimming", "IL2070",
    Justification = "Method/lambda Type parameters trace back to GetType(), Expression.Type, or GetInterfaces() on concrete runtime types. Reflected members are present at expression-tree build time.")]

And further on, above var converter = TypeDescriptor.GetConverter(actualTargetType);:

  // TypeDescriptor provides string→T conversion for types with a registered TypeConverter
  // (e.g. Color, Point, or user types with [TypeConverter(...)]) that Convert.ChangeType
  // cannot handle. In trimmed builds the converter may be removed, causing CanConvertFrom
  // to return false and falling through safely to Convert.ChangeType below.
  #pragma warning disable IL2026, IL2067 // TypeDescriptor.GetConverter uses RequiresUnreferencedCode; actualTargetType is a runtime type, not statically resolvable

[w-ahmad]

I think we shouldn't suppress these warnings, as they let users know that TableView uses reflection. All we can do is use the provided supportive attributes to display meaningful warning messages in the logs.

I understand your idea of using source generators to remove reflection from code, which would make the code fully AOT compatible. I experimented with source generators once but found some major breaking changes, so I postponed it for later, though I haven’t completely dropped the idea. see #349

[DanielTrommel]

Yes, understandable. This PR is imo then ready to be merged.

…

On Thu, 25 Jun 2026, 11:27 Waheed Ahmad, ***@***.***> wrote: *w-ahmad* left a comment (w-ahmad/WinUI.TableView#382) <#382 (comment)> I think we shouldn't suppress these warnings, as they let users know that TableView uses reflection. All we can do is use the provided supportive attributes to display meaningful warning messages in the logs. I understand your idea of using source generators to remove reflection from code, which would make the code fully AOT compatible. I experimented with source generators once but found some major breaking changes, so I postponed it for later, though I haven’t completely dropped the idea. see #349 <#349> — Reply to this email directly, view it on GitHub <#382?email_source=notifications&email_token=AEYV7OF22L5JTI4K74L7DRT5BTV6JA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZZG43TIOJQGE3KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4797749016>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEYV7ODIBYHFWY4DBXIAFTL5BTV6JAVCNFSNUABFKJSXA33TNF2G64TZHM3TIOJVGUYDGOBXHNEXG43VMU5TINRUGQYDAMJTGQZKC5QC> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AEYV7OGFMKSZN63IMNCJTTL5BTV6JA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZZG43TIOJQGE3KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android <https://github.com/notifications/mobile/android/AEYV7OGCA6N55MIR5QN5LST5BTV6JA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZZG43TIOJQGE3KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today! You are receiving this because you were mentioned.Message ID: ***@***.***>