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.

Sign up for GitHub

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

Jump to bottom

Operators.( .. .. ) throws an ArgumentException when ^Step does not implement IComparable #6238

Open
JanWosnitza opened this issue Feb 16, 2019 · 6 comments · May be fixed by #17133
Open

Operators.( .. .. ) throws an ArgumentException when ^Step does not implement IComparable #6238

JanWosnitza opened this issue Feb 16, 2019 · 6 comments · May be fixed by #17133
Assignees
@abelbraaksma
Labels
Area-Library Issues for FSharp.Core not covered elsewhere Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone
Backlog

Comments

@JanWosnitza
Copy link
Contributor

Operators.( .. .. ) throws an ArgumentException when ^Step does not implement non-generic System.IComparable.

Repro steps

Run the following code:

type Step(value:int) =
    member __.Value = value
    
    static member Zero = Step 0

type Value(value:int) =
    member __.Value = value
    
    static member (+)(a:Value, b:Step) = Value(a.Value + b.Value)
    
    interface System.IComparable with
        member a.CompareTo(b) =
            match b with
            | :? Value as b' -> compare a.Value b'.Value
            | _ -> failwith "unsupported"
    
[Value 0 .. Step 2 .. Value 2]

Expected behavior

I would expect a compiler error. Similar to

  • Step.Zero not implemented
  • Value.(+) not implemented
  • Value not implementing System.IComparable

Actual behavior

The last line throws this exception:

System.ArgumentException: Failure during generic comparison: the type 'FSI_0006+Step' does not implement the System.IComparable interface. This error may be arise from the use of a function such as 'compare', 'max'
or 'min' or a data structure such as 'Set' or 'Map' whose keys contain instances of this type.
   at Microsoft.FSharp.Core.LanguagePrimitives.HashCompare.FailGenericComparison[a](Object obj)
   at Microsoft.FSharp.Core.LanguagePrimitives.HashCompare.GenericCompare(GenericComparer comp, Object xobj, Object yobj)
   at Microsoft.FSharp.Core.LanguagePrimitives.HashCompare.GenericLessThanIntrinsic[T](T x, T y)
   at Microsoft.FSharp.Core.Operators.OperatorIntrinsics.gen@4848-3[TStep,T](TStep zero, FSharpFunc`2 add, T start, TStep step, T stop, Unit unitVar0)
   at Microsoft.FSharp.Core.Operators.OperatorIntrinsics.RangeStepGeneric@4936-1.System-Collections-Generic-IEnumerable`1-GetEnumerator()
   at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.MoveNextImpl()
   at Microsoft.FSharp.Collections.SeqModule.ToList[T](IEnumerable`1 source)
   at <StartupCode$FSI_0006>.$FSI_0006.main@()

Known workarounds

Run the code and realize you need to implement the interface System.IComparable on Step:

    interface System.IComparable with
        member a.CompareTo(b) = 
            match b with
            | :? Step as b' -> compare a.Value b'.Value
            | _ -> failwith "unsupported"

Or not using the start .. step .. finish operator, but generate the sequence somehow different (where step is not required to be comparable to Zero at runtime).

Related information

I had this issue when using 2 structs defined in C# and tried to iterate over them.

  • Microsoft (R) F# Interactive version 10.2.3 for F# 4.5
  • Microsoft Windows 10
@cartermp cartermp added this to the Unknown / not bug milestone Mar 6, 2020
@dsyme dsyme added Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Bug Area-Library Issues for FSharp.Core not covered elsewhere Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. and removed Area-Compiler Feature Improvement Area-Compiler-Checking Type checking, attributes and all aspects of logic checking labels Apr 1, 2022
@brianrourkeboll
Copy link
Contributor

This seems odd at first glance. The implementation of RangeStepGeneric in prim-types.fs, which is called in this case for (.. ..), uses =, >, and < on step, which would normally force the equality and comparison constraints to bubble up:

fsharp/src/FSharp.Core/prim-types.fs

Lines 5790 to 5796 in fc42ec9

let inline integralRangeStepEnumerator (zero,add,n,step,m,f) : IEnumerator<_> =
// Generates sequence z_i where z_i = f (n + i.step) while n + i.step is in region (n,m)
if n = m then
new SingletonEnumerator<_> (f n) |> enumerator
else
let up = (n < m)
let canStart = not (if up then step < zero else step > zero) // check for interval increasing, step decreasing

fsharp/src/FSharp.Core/prim-types.fs

Lines 5942 to 5944 in fc42ec9

let inline integralRangeStep<'T,'Step> (zero:'Step) (add:'T -> 'Step -> 'T) (n:'T, step:'Step, m:'T) =
if step = zero then invalidArg "step" (SR.GetString(SR.stepCannotBeZero));
let gen() = integralRangeStepEnumerator (zero, add, n, step, m, id)

fsharp/src/FSharp.Core/prim-types.fs

Line 6032 in fc42ec9

let RangeStepGeneric zero add start step stop : seq<'T> = integralRangeStep zero add (start,step,stop)

fsharp/src/FSharp.Core/prim-types.fs

Lines 6911 to 6912 in fc42ec9

let inline (.. ..) (start: ^T) (step: ^U) (finish: ^T) =
RangeStepGeneric (GenericZero< (^U) >) Checked.(+) start step finish

The signature for (.. ..) in prim-types.fsi, however, specifies neither the equality constraint nor the comparison constraint for ^Step, unless ^Step is unified with ^T. I wonder if that plays into it at all?

fsharp/src/FSharp.Core/prim-types.fsi

Lines 3756 to 3762 in fc42ec9

val inline (.. ..): start: ^T -> step: ^Step -> finish: ^T -> seq< ^T >
when (^T or ^Step): (static member (+): ^T * ^Step -> ^T)
and ^Step: (static member Zero: ^Step)
and ^T: equality
and ^T: comparison
and default ^Step: ^T
and default ^T: int

It would be a compile-time breaking change to update the signature for (.. ..) to include and ^Step: equality and ^Step: comparison, but since any existing code where ^Step doesn't conform to those constraints already fails at runtime, maybe it could be done?

@abelbraaksma
Copy link
Contributor

It seems like auto-inference doesn't add the and ^Step: comparison. Normally that happens automatically if you use it (and it is used, as you mention):

> let foo a b = a > b;;
val foo: a: 'a -> b: 'a -> bool when 'a: comparison

But also note that the *.fsi file has and default ^Step: ^T. This should be enough to infer that ^Step requires equality and comparison as well. Is this a bug in using the default ^X: ^Y syntax?

@abelbraaksma
Copy link
Contributor

abelbraaksma commented May 8, 2024 •
edited

@brianrourkeboll, I don't know if you tested the OP's example code, but I just tried it with the latest from main and it throws another tantrum:

error FS0193: The member or object constructor 'op_Addition' is not public. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.

image

This member is clearly accessible...

This, I think, is a regression. I cannot repro this with the latest RTM of F#.

@brianrourkeboll
Copy link
Contributor

I don't know if you tested the OP's example code, but I just tried it with the latest from main and it throws another tantrum:

error FS0193: The member or object constructor 'op_Addition' is not public. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.

Hmm, I do not get that when using a VSIX built from main in the last week or two.

@abelbraaksma
Copy link
Contributor

abelbraaksma commented May 8, 2024 •
edited

Ok, this is weird.

I pasted it under module internal RangeTestsHelpers = in the test file PrimTypes.fs. Now, I would argue that any internal code should be accessible from other internal code, but this is likely a different issue and should be reported separately.

Once I put the code in a public module, it worked as expected.

Note that, if I change the definition to be this (as you suggested):

val inline (.. ..): start: ^T -> step: ^Step -> finish: ^T -> seq< ^T >    
                        when (^T or ^Step): (static member (+): ^T * ^Step -> ^T) 
                        and ^Step: (static member Zero: ^Step)
                        and ^T: equality
                        and ^T: comparison                                
                        and ^Step: equality
                        and ^Step: comparison                                
                        and default ^Step: ^T
                        and default ^T: int

it will behave as the OP suggested (though the error is thrown 8 times???)

error FS0193: The type 'Step' does not support the 'comparison' constraint. For example, it does not support the 'System.IComparable' interface

@brianrourkeboll
Copy link
Contributor

I pasted it under module internal RangeTestsHelpers = in the test file PrimTypes.fs. Now, I would argue that any internal code should be accessible from other internal code, but this is likely a different issue and should be reported separately.

That seems to be #16762.

abelbraaksma added a commit to abelbraaksma/fsharp that referenced this issue May 10, 2024
@abelbraaksma
equality & comparison constraints for ^Step param in .. .. operator
539ba22 
Fixes dotnet#6238. Compiler does not infer this automatically through `and default`.
@abelbraaksma abelbraaksma linked a pull request May 10, 2024 that will close this issue
Draft
4 tasks
abelbraaksma added a commit to abelbraaksma/fsharp that referenced this issue May 10, 2024
@abelbraaksma
equality & comparison constraints for ^Step param in .. .. operator
05f1282 
Fixes dotnet#6238. Compiler does not infer this automatically through `and default`.
@abelbraaksma abelbraaksma self-assigned this May 15, 2024
abelbraaksma added a commit to abelbraaksma/fsharp that referenced this issue May 23, 2024
@abelbraaksma
equality & comparison constraints for ^Step param in .. .. operator
2de9d1a 
Fixes dotnet#6238. Compiler does not infer this automatically through `and default`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@abelbraaksma abelbraaksma

Labels
Area-Library Issues for FSharp.Core not covered elsewhere Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
F# Compiler and Tooling
Status: New
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

Add constraints for ^Step param type in ranges in (.. ..) operators abelbraaksma/fsharp
5 participants
@JanWosnitza @cartermp @dsyme @brianrourkeboll @abelbraaksma