Skip to content

Conversation

@fabiomadge
Copy link
Collaborator

@fabiomadge fabiomadge commented May 11, 2025

Fixes #6181.

Comment on lines 587 to 594
if (allowCommonSuperType) {
if (HaveCommonSuperPreType(aa, bb)) {
return true;
}
} else {
if (IsSuperPreTypeOf(aa, bb) || IsSuperPreTypeOf(bb, aa)) {
return true;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another way to accomplish this is to leave the original comparison

if (IsSuperPreTypeOf(aa, bb) || IsSuperPreTypeOf(bb, aa)) {
        return true;
}

and then do

if (allowCommonSuperType && HaveCommonSuperPreType(aa, bb)) {
  return true;
}

where the initial comparison if (IsSuperPreTypeOf(t, u) || IsSuperPreTypeOf(u, t)) ... inside HaveCommonSuperPreType can be removed.

@fabiomadge fabiomadge marked this pull request as ready for review May 16, 2025 22:08
Copy link
Contributor

@ssomayyajula ssomayyajula left a comment

Choose a reason for hiding this comment

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

Ah OK so now two types are comparable iff, additionally to the usual rules, they have a common supertype and allowCommonSuperType is true. And allowCommonSuperType is true everywhere except for conversions and type tests. But if I do a quick search through PreTypeResolve.Expressions.cs, I never see both allowConversion and allowCommonSuperType to be both true or false at the same time, so isn't one implied by the other? Or did I miss something?

@fabiomadge
Copy link
Collaborator Author

You're right, we could do with just one boolean flag the way we use this method. I think of them as two independent relaxations of the basic rule, so I like exposing them as such, but I'm happy to merge the flags into one.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New resolver is too restrictive when deciding on comparability of types

3 participants