Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DataGrid native aot crash when sorting. #17248

Merged
merged 5 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,29 @@ private IComparer GetComparerForType(Type type)
if (type == typeof(string))
return _cultureSensitiveComparer.Value;
else
return GetComparerForNotStringType(type);
}

internal static IComparer GetComparerForNotStringType(Type type)
{
#if NET6_0_OR_GREATER
if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>) && type.GetGenericArguments()[0].IsAssignableTo(typeof(IComparable)))
return Comparer<object>.Create((x, y) =>
{
if (x == null)
return y == null ? 0 : -1;
else
return (x as IComparable)!.CompareTo(y);
});
else if (type.IsAssignableTo(typeof(IComparable)))
return Comparer<object>.Create((x, y) => (x as IComparable)!.CompareTo(y));
else
return Comparer<object>.Create((x, y) => 0); //avoid using reflection to avoid crash on AOT
Copy link
Member

Choose a reason for hiding this comment

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

On NET6_0_OR_GREATER you can check for this property whether reflection is supported: https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.runtimefeature.isdynamiccodesupported?view=net-8.0

On AOT builds IsDynamicCodeSupported returns false, and this fallback would make sense there.

Copy link
Member

@maxkatz6 maxkatz6 Oct 11, 2024

Choose a reason for hiding this comment

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

This way we also won't regress any application that relies on this reflection sorting right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice suggestion. Done.

#else
return (typeof(Comparer<>).MakeGenericType(type).GetProperty("Default")).GetValue(null, null) as IComparer;
#endif
}

private Type GetPropertyType(object o)
{
return o.GetType().GetNestedPropertyType(_propertyPath);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,223 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using Avalonia.Collections;
using Avalonia.Logging;
using Xunit;

namespace Avalonia.Controls.DataGridTests.Collections;

public class NoStringTypeComparerTests
{
public static IEnumerable<object[]> GetComparerForNotStringTypeParameters()
{
yield return
[
nameof(Item.IntProp1),
(object item, object value) =>
{
(item as Item)!.IntProp1 = (int)value;
},
(object item) => (object)(item as Item)!.IntProp1,
new object[] { 2, 3, 1 },
new object[] { 1, 2, 3 }
];
yield return
[
nameof(Item.IntProp2),
(object item, object value) =>
{
(item as Item)!.IntProp2 = (int?)value;
},
(object item) => (object)(item as Item)!.IntProp2,
new object[] { 2, 3, null, 1 },
new object[] { null, 1, 2, 3 }
];
yield return
[
nameof(Item.DoubleProp1),
(object item, object value) =>
{
(item as Item)!.DoubleProp1 = (double)value;
},
(object item) => (object)(item as Item)!.DoubleProp1,
new object[] { 2.1, 3.1, 1.1 },
new object[] { 1.1, 2.1, 3.1 }
];
yield return
[
nameof(Item.DoubleProp2),
(object item, object value) =>
{
(item as Item)!.DoubleProp2 = (double?)value;
},
(object item) => (object)(item as Item)!.DoubleProp2,
new object[] { 2.1, 3.1, null, 1.1 },
new object[] { null, 1.1, 2.1, 3.1 }
];
yield return
[
nameof(Item.DecimalProp1),
(object item, object value) =>
{
(item as Item)!.DecimalProp1 = (decimal)value;
},
(object item) => (object)(item as Item)!.DecimalProp1,
new object[] { 2.1M, 3.1M, 1.1M },
new object[] { 1.1M, 2.1M, 3.1M }
];
yield return
[
nameof(Item.DecimalProp2),
(object item, object value) =>
{
(item as Item)!.DecimalProp2 = (decimal?)value;
},
(object item) => (object)(item as Item)!.DecimalProp2,
new object[] { 2.1M, 3.1M, null, 1.1M },
new object[] { null, 1.1M, 2.1M, 3.1M }
];
yield return
[
nameof(Item.EnumProp1),
(object item, object value) =>
{
(item as Item)!.EnumProp1 = (LogEventLevel)value;
},
(object item) => (object)(item as Item)!.EnumProp1,
new object[] { LogEventLevel.Information, LogEventLevel.Debug, LogEventLevel.Error },
new object[] { LogEventLevel.Debug, LogEventLevel.Information, LogEventLevel.Error }
];
yield return
[
nameof(Item.EnumProp2),
(object item, object value) =>
{
(item as Item)!.EnumProp2 = (LogEventLevel?)value;
},
(object item) => (object)(item as Item)!.EnumProp2,
new object[]
{
LogEventLevel.Information,
LogEventLevel.Debug,
null,
LogEventLevel.Error
},
new object[]
{
null,
LogEventLevel.Debug,
LogEventLevel.Information,
LogEventLevel.Error
}
];
yield return
[
nameof(Item.CustomProp2),
(object item, object value) =>
{
(item as Item)!.CustomProp2 = (CustomType?)value;
},
(object item) => (object)(item as Item)!.CustomProp2,
new object[]
{
new CustomType() { Prop = 2 },
new CustomType() { Prop = 3 },
null,
new CustomType() { Prop = 1 }
},
new object[]
{
null,
new CustomType() { Prop = 1 },
new CustomType() { Prop = 2 },
new CustomType() { Prop = 3 }
}
];
}

[Theory]
[MemberData(nameof(GetComparerForNotStringTypeParameters))]
public void GetComparerForNotStringType_Correctly_WhenSorting(
string pathName,
Action<object, object> setAction,
Func<object, object> getAction,
object[] orignal,
object[] ordered
)
{
List<Item> items = new();
for (int i = 0; i < orignal.Length; i++)
{
var item = new Item();
setAction(item, orignal[i]);
items.Add(item);
}

//Ascending
var sortDescription = DataGridSortDescription.FromPath(
pathName,
ListSortDirection.Ascending
);
sortDescription.Initialize(typeof(Item));
var result = sortDescription.OrderBy(items).ToList();

for (int i = 0; i < ordered.Length; i++)
{
Assert.Equal(ordered[i], getAction(result[i]));
}

//Descending
sortDescription = DataGridSortDescription.FromPath(pathName, ListSortDirection.Descending);
sortDescription.Initialize(typeof(Item));
result = sortDescription.OrderBy(items).ToList();

ordered = ordered.Reverse().ToArray();
for (int i = 0; i < ordered.Length; i++)
{
Assert.Equal(ordered[i], getAction(result[i]));
}
}

private class Item
{
public int IntProp1 { get; set; }
public int? IntProp2 { get; set; }
public double DoubleProp1 { get; set; }
public double? DoubleProp2 { get; set; }

public decimal DecimalProp1 { get; set; }
public decimal? DecimalProp2 { get; set; }

public LogEventLevel EnumProp1 { get; set; }
public LogEventLevel? EnumProp2 { get; set; }
public CustomType? CustomProp2 { get; set; }
}

public struct CustomType : IComparable
{
public int Prop { get; set; }

public int CompareTo(object obj)
{
if (obj is CustomType other)
{
return Prop.CompareTo(other.Prop);
}
else
{
return 1;
}
}

public override bool Equals(object obj)
{
if (obj is CustomType other)
{
return Prop == other.Prop;
}
return false;
}
}
}
Loading