Deprecate non-zero-based Array2Ds, fixing trimmability

[charlesroddie]

The Array2D module is designed around allowing for non-zero-based Array2Ds. This is very unusual in the following ways:

• It is not the case for the Array, Array3D, and Array4D modules. E.g. for Array: "Arrays are fixed-size, zero-based, mutable collections...". So it's an odd one out.
• It is unusual in dotnet, and C# does not have syntax for creating non-zero-based Array2Ds, or a normal constructor.
• It is also very unusual in F#. A github search for Array2D.initBased brings up 101 files, compared to 2.6k for Array2D.init, and almost all the examples are documentation or copies of the F# module, the exceptions being teaching code like advent of code or exercism.

This causes problems in F# code in that most Array2D module functions need to allow for the possibility of arrays being non-zero-based, and so may end up calling Array2D.initBased, and this uses a non-trim-safe method (Array.CreateInstance(typeof<'T>, [|length1;length2|], [|base1;base2|]) :?> 'T[,]) which has RequiresDynamicCodeAttribute. This results in trim warnings for F# code using Array2D functions.

It also has caused random bugs in our app in the past in ways that are platform dependent (e.g. we had an Android bug with System.MethodAccessException from Array2DModule.InitializeBased[SkViews.Table.Cell] resulting from normal use of a private type.

I propose we deprecate non-zero-based Array2Ds as an anomaly.

This would involve:

• Mark zeroCreateBased, createBased, initBased, rebase as deprecated
• Fail other methods if bases are non-zero

The existing way of approaching this problem in F# is ...

Avoid the trim issues by avoiding the FSharp.Core module and defining another zero-based module for Array2D.

Pros and Cons

The advantages of making this adjustment to F# are ...

Consistency between Array2D and Array, Array3D and Array4D.
Avoiding allowing users to create things that are not expected in dotnet.

The disadvantages of making this adjustment to F# are ...

A small amount of existing code using the based methods would need to be migrated away or would need to make copies of old methods.

Extra information

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

S

Related suggestions: (put links to related suggestions here)

Affidavit (please submit!)

Please tick these items by placing a cross in the box:

• [ x ] This is not a question (e.g. like one you might ask on StackOverflow) and I have searched StackOverflow for discussions of this issue
• [ x ] This is a language change and not purely a tooling change (e.g. compiler bug, editor support, warning/error messages, new warning, non-breaking optimisation) belonging to the compiler and tooling repository
• [ x ] This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it
• [ x ] I have searched both open and closed suggestions on this site and believe this is not a duplicate

Please tick all that apply:

• This is not a breaking change to the F# language design
• [ x ] I or my company would be willing to help implement and/or test this

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.

[T-Gro]

1. Mark as deprecated with explanation, but keep working
2. Offer copy-pasteable code to migrate (so that people can use their own local version of Array2D)
3. Deprecated -> Removed at a later version

If I understand the issue correctly, an intermmediate need should be resolved by rewriting the following two methods to NOT call the based variants, or?

    [<CompiledName("Create")>]
    let create length1 length2 (value:'T) = 
        createBased 0 0 length1 length2 value

    [<CompiledName("Initialize")>]
    let init length1 length2 initializer = 
        initBased 0 0 length1 length2 initializer

Also likely, if the based variants were marked as inline, optimizer should be able to strip the code path for dynamics access and also work (needs verification though). The initializer based one might then also be changed to make use of InlineIfLambda.

(right now they are not inline, so even for the 0 0 base scenario the trim warning detector still sees the calls to Array.CreateInstance)

[smoothdeveloper]

I don't understand the need to deprecate. Non zero-based arrays are useful in my book, when you port code that is non zero based, and you want to avoid off-by-something errors, this feature is a god send.

AFAIR, Fortran supports non-zero based indices (it is one-based by default).

In fact, we should have ability to have that for 1D arrays.

If we consider doing something, it could be to have those API be in a separate FSharp.Core.Something package, still officially supported, but just optional; this is along with modularization of FSharp.Core which has been long standing and relevant topic for several reasons.

[smoothdeveloper]

I'm curious, how does this feature actually work? it seems the arrays are regular CLR arrays, yet, F# does some tracking to adjust the indices based on the initialisation of it.

[T-Gro]

Yes, the information is stored in the System.Array type itself.
And at creation + a few use sites, FSharp.Core adjusts index access based on the base number per each rank.

Separate .dll and/or separate module.
A safety-first approach would be to wrap such arrays into a different type, to avoid passing them into existing array-based functions which might crash.

[smoothdeveloper]

I see; GetLowerBound returns the base, I didn't know BCL array API had ability to store the index-base.

I see even less reasons to do anything, but maybe consider splitting some of those APIs in separate libraries to get FSharp.Core lighter.

If we consider things from type safety standpoint, it seems F# should rather leverage similar infrastructure than what makes UoM work, meaning something that gets erased, but still allow to catch type errors; albeit it should still allow passing to methods that are not annotated.

It is possible to have safe code by using GetLowerBound API rather than assuming the magic number is 0 (or 42...) for array index (which is a bit like a religious thing).

[T-Gro]

Possible yes, but you might be losing valuable JIT-time optimization, especially to avoid bound checks.
Not because it is impossible, but because 0-based is so predominant and therefore optimized for.

[charlesroddie]

@smoothdeveloper if you want a non-zero-based collection type there is not much support in dotnet. It's only through ArrayN and only via a reflection-based backdoors not normal constructors. Anything else, including immutable collections like FSharpList and ImmutableArray, is zero-based.

But creating new collection types is easy so if you want ArrayBased, ImmArrayBased, Array2DBased, ImmArray2D, ImmArray2DBased etc. very quickly and I'd support creating things like this and putting this in an FSharp.ExtraCollections or BasedCollections package.

@T-Gro If I understand the issue correctly, an intermediate need should be resolved by rewriting the following two methods to NOT call the based variants, or?

Yes some module functions like init are fixable but others like map I don't think are fixable in this way.