-
Notifications
You must be signed in to change notification settings - Fork 21
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
Warning when comparing sequences #1387
Comments
Probably good case for analyzer |
What is the implementation of this? I thought that However this does not explain why the following is true:
That is very surprising to me. |
It's delegating to the concrete type's Here's a trivial example that doesn't involve the special dual-purpose `seq` value (which can be both a normal F# function and something like a computation expression builder).type IFace = interface end
let iface x = x :> IFace
type A (x : int) =
member _.X = x
interface IFace
iface (A 3) = iface (A 3) // false
type B (x : int) =
member _.X = x
interface IFace
override this.Equals obj = match obj with :? B as other -> this.X = other.X | _ -> false
override this.GetHashCode () = this.X
iface (B 3) = iface (B 3) // true
It would unfortunately probably be rather difficult to create a robust analyzer for this — you'd somehow need to track a value's concrete type from instantiation through to any usage that might call
|
Or rather have which forces all operators to work with structural comparison semantics if collections are detected? |
I propose we issue a warning when comparing sequences.
As of now the behaviour of equals/comparison on sequences depends on use of constants and/or what the sequence is backed by.
Another example is
Seq.groupBy
that in some cases will produce unexpected results.Pros and Cons
The advantage of making this adjustment to F# is that it would be more clear that equality between two sequences does not always use structural comparison.
A disadvantage of making this adjustment to F# is that the source of the warning may be confusing (as in the case of
Seq.groupBy
.Extra information
Estimated cost (XS, S, M, L, XL, XXL): S
Affidavit (please submit!)
Please tick these items by placing a cross in the box:
Please tick all that apply:
For Readers
If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.
The text was updated successfully, but these errors were encountered: