Skip to content

fix: change M0061 and M0062 from warnings to errors #4926

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rvanasa
Copy link
Contributor

@rvanasa rvanasa commented Feb 27, 2025

This PR promotes M0061 ("comparing abstract type to itself at supertype") and M0062 ("comparing incompatible types at common supertype") from warnings to errors.

This is motivated by the unintuitive behavior of comparing at supertype Any, which always returns true. For instance, comparing 5 and ?4 returns true and therefore can result in buggy behavior or false positive test cases.

@rvanasa rvanasa requested a review from a team as a code owner February 27, 2025 19:31
@crusso
Copy link
Contributor

crusso commented Feb 27, 2025

I think this need some more thought. I think some use case might still be sensible but not sure.

@rvanasa rvanasa marked this pull request as draft February 27, 2025 20:10
@rvanasa
Copy link
Contributor Author

rvanasa commented Feb 27, 2025

Here is a counterexample where a developer might want to be able to compare at a supertype:

type A = { x : Int; y : Int};
type B = { x : Int; z : Int};

let a : A = { x = 0; y = 1 };
let b : B = { x = 1; z = 2 };

a == b // => false, supertype warning

Output:

false : Bool
mo:7.1-7.7: warning [M0062], comparing incompatible types
  {x : Int; y : Int}
and
  {x : Int; z : Int}
at common supertype
  {x : Int}

Maybe we could remove the warning or at least have the same behavior as comparing with a subtype. Here is the current behavior for a similar case:

type A = { x : Int};
type B = { x : Int; z : Int};

let a : A = { x = 0 };
let b : B = { x = 1; z = 2 };

a == b // => false, no warning

Another option could be to error for types such as Any, (), {}, etc.

Motivating example cases:

{ a = 1 } == { a = -1 }; // => true, warning
{ a = 1 } == { b = 2 }; // => true, warning

@rvanasa rvanasa changed the title Change M0061 and M0062 from warnings to errors fix: change M0061 and M0062 from warnings to errors Feb 28, 2025
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.

2 participants