fsharp / fslang-suggestions

The place to make suggestions, discuss and vote on F# language and core library features
346 stars 21 forks source link

[Slice] Allow slice syntax to use instance `Slice` method instead of requiring `GetSlice` method #1317

Open xperiandri opened 1 year ago

xperiandri commented 1 year ago

I propose we convert slice syntax not only to GetSlice call but to Slice call that is now implemented by all collections

The existing way of approaching this problem in F# is implementing SRTP extension method

namespace System

open System.Runtime.CompilerServices

[<Extension>]
type SliceExtensions =

    [<Extension>]
    static member inline GetSlice<'c
            when 'c : (member Slice: int * int -> 'c)
            and  'c : (member Length: int)>
        (source: 'c, from: int option, ``to``: int option) =

        let from = from |> Option.defaultValue 0
        let ``to`` = ``to`` |> Option.defaultValue (source.Length - 1)
        source.Slice(from, ``to`` + 1 - from)

Pros and Cons

The advantages of making this adjustment to F# are all slicing available in C# will work in F#

The disadvantages of making this adjustment to F# are not exist

Extra information

Estimated cost (XS, S, M, L, XL, XXL): XS

Related suggestions:

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 :+1: emoji on this issue. These counts are used to generally order the suggestions by engagement.

smoothdeveloper commented 1 year ago

I know GetSlice is kind of fresh, is there a sense that now "the BCL team has decided the name", we could deprecate GetSlice through a warning, and have Slice instead?

It seems the "framework design guidelines" trend of everything being a bit verbose and explicit, "GetX", are past history, and the BCL design team is looking at C++ (https://en.cppreference.com/w/cpp/numeric/valarray/slice) for choosing identifiers more than those original guidelines.

Maybe that will help us making the same, if we face again "let's do transversal feature that carries a name into F# libraries" but can't wait for BCL team to decide, we can pick from C++ if the concept exists there and is applicable.

jwosty commented 1 year ago

I run into this most frequently with Span.

brianrourkeboll commented 8 months ago

Is there a good reason not to add support for Slice?

The only downsides I can think of are:

But otherwise it seems silly to make everyone write an extension method to use the BCL collections' built-in slicing capabilities, forever, when the compiler could just do it for them.

dsyme commented 8 months ago

I'll mark this as approved in principle. It should be possible define Slice and have the F# slicing syntax light up.

Equally the SRTP workaround is pretty good!

brianrourkeboll commented 4 months ago

So it turns out that this is closely related to #1044, because the C# spec for slicing and ranges follows these rules:

  1. First, look for an indexer taking System.Range; if found, use that.
  2. Otherwise, look for a Slice(int, int) method.

That is, the Slice method is not called if there is also a Range indexer.

This means that if we want to support Slice in F#, we at the very least also need to be aware of indexers taking System.Range.

If we went ahead with adding support for Slice alone, and if a given type had both a System.Range indexer and a Slice method, then using slicing syntax on it in C# would call the System.Range indexer, while using slicing syntax on it in F# would call the Slice method.

Presumably this disparity would be undesirable, since this[Range] and Slice could have entirely different behaviors, return types, etc. Changing our minds about this later would be a breaking change.

This problem holds even though the BCL itself has only added Slice methods and has not added any Range indexers, because this would still affect any user-defined types that have both this[Range] and Slice(int, int).

Ways forward

There are a few different ways forward here. Of the following, the first is straightforward (I have it working); the second is doable (I have it mostly working); the third is more involved.

Detect Range indexers, but don't support using them with slicing syntax

In theory, we could still implement this suggestion (supporting Slice) without making any further decisions about System.Range or System.Index by simply detecting whether an indexer taking System.Range is present. If one is present, we would raise an error (just as happens now), whether or not a Slice method is also present. If we later added full support for System.Range, we would then simply start calling the Range indexer for such types.

Support using Range indexers with slicing syntax without (yet) adding full support for System.Range itself

Alternatively, we could add only enough support for System.Range and System.Index to make this feature work.

In the compiler, we could look for a System.Range indexer, and, if found, look for its new : Index * Index -> Range constructor. We could then look for the necessary members on Index to help construct the appropriate Range value from the F# start and/or finish int values. The compiler would simply emit calls to the constructors and methods it found with the appropriate paths, names, and signatures, and pass the constructed Range into the indexer.

We could do this without actually supporting System.Range or System.Index anywhere else.

Fully support System.Range and System.Index

Third, we could write an RFC and implementation for #1044, and probably also revisit RFC FS-1076.

This would probably be good to do eventually, although I don't know if I'm willing to do all of this myself right now, unless someone is willing to sponsor me :)

brianrourkeboll commented 4 months ago

So I have done some more experimenting, and it is feasible to add support for Slice, System.Index and System.Range indexers, and from-end indexing/slicing (i.e., a new approach to RFC FS-1076) all at once — with the following questions and caveats:[^1]

Support for Slice

Support for indexed properties taking System.Index and System.Range

Support for from-end indexing/slicing

[^1]: Yes, I'm eliding many details. I hope to be able to get an RFC organized soon.

nodakai commented 1 month ago

Could we support int voption as well? (maybe even int64 voption)

brianrourkeboll commented 1 month ago

@nodakai

Could we support int voption as well? (maybe even int64 voption)

Are you asking that we update the existing support for GetSlice to recognize voption? I think that should be a separate suggestion.

The support for Slice wouldn't involve options (value or reference) at all.

vzarytovskii commented 1 month ago

@nodakai

Could we support int voption as well? (maybe even int64 voption)

Are you asking that we update the existing support for GetSlice to recognize voption? I think that should be a separate suggestion.

The support for Slice wouldn't involve options (value or reference) at all.

Yeah, should be a new suggestion, but I don't think we want to have any implicit conversions from/to options.