Fixes to support multi-dimensional arrays #27618
Conversation
| if (typeof(IStructuralEquatable).IsAssignableFrom(type)) | ||
| // We exclude multi-dimensional arrays even though they're IStructuralEquatable because of | ||
| // https://github.com/dotnet/runtime/issues/66472 | ||
| if (typeof(IStructuralEquatable).IsAssignableFrom(type) && !(type.IsArray && typeof(T).GetArrayRank() != 1)) |
There was a problem hiding this comment.
Note: this means that multidimensional arrays are excluded from structural equals checks. Assuming the runtime-side issue is fixed one day, it would be a breaking change to remove this exclusion, so we may be better off not doing this and telling users to set up a value converter explicitly.
There was a problem hiding this comment.
I think this is fine; I think an explicit value comparer is the way to go for this.
There was a problem hiding this comment.
Agree, will remove this specific exception - but will wait a few days before merging to see responses to dotnet/runtime#66472.
There was a problem hiding this comment.
OK, no movement on the runtime side - I'll remove the check for muiltidimensional arrays; this means they will cause an exception by default, and can be handled via an explicit value comparer.
roji
force-pushed
the
MultidimensionalArrays
branch
from
fbe976b
to
5b2b58c
Compare
|
Hello @roji! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
|
Apologies, while this PR appears ready to be merged, I've been configured to only merge when all checks have explicitly passed. The following integrations have not reported any progress on their checks and are blocking auto-merge:
These integrations are possibly never going to report a check, and unblocking auto-merge likely requires a human being to update my configuration to exempt these integrations from requiring a passing check. Give feedback on thisFrom the bot dev teamWe've tried to tune the bot such that it posts a comment like this only when auto-merge is blocked for exceptional, non-intuitive reasons. When the bot's auto-merge capability is properly configured, auto-merge should operate as you would intuitively expect and you should not see any spurious comments. Please reach out to us at fabricbotservices@microsoft.com to provide feedback if you believe you're seeing this comment appear spuriously. Please note that we usually are unable to update your bot configuration on your team's behalf, but we're happy to help you identify your bot admin. |
ghost
merged commit 
|
Thank you for your commit. |
Fixes #26975