Runtime: "ItemRef" - Ref element accessor for types that already have an indexer.

[VSadov]

We have a number of existing indexable types that could benefit from a ref-returning element accessor. For example ImmutableArray.

A typical problem is that such types already have a get/set indexer, so the only way to add an indexed byref access to elements is to add a ref returning method.
I believe this will happen one way or another, since there is a motivation. So, I think we should standardize on the name of such method.

Here I want to propose the following convention:

```C#
// when container is readonly - I.E. ImmutableArray
public ref readonly T ItemRef(int index);

    // when container is writeable - I.E. List<T>
    public ref T ItemRef(int index);

The alternatives could be 

-   “ElementAt” – often already taken
-   “Item”          –  clashes with the underlying name of the indexer
-    “Address”    - could work too, but I think “ItemRef” reads a lot better.
-   “ItemAt”       –  original proposed name, "ItemRef" still seems better

Examples of uses:

```C#
    public class Program
    {
        static void Main(string[] args)
        {
            var o = new MyCollection<int>();

            // use for assignment
            o.ItemRef(1) = 42;

            // bind to a ref local
            ref var first = ref o.ItemRef(1);
            first = 42;

            // read indirectly. (assume the API takes "in" parameter)
            Print(o.ItemRef(1));
        }
    }

    class MyCollection<T>
    {
        public ref T ItemRef(int index) => throw null;
    }

Once we agree on the name/pattern we can enter separate bugs for particular APIs.

[VSadov]

CC: @stephentoub @KrzysztofCwalina @jaredpar

[benaadams]

Sounds good

Address isn't a very .NET type of name; ItemRef/ItemReference would match it more, also clashes a bit with VB.NET's AddressOf

ItemAt I think is good though.

[benaadams]

Was a suggestion of xByRef for Dictionary https://github.com/dotnet/corefx/issues/21240, https://github.com/dotnet/corefx/issues/20684

Dictionary<TKey, TVal>
{
    public ref TValue GetValueByRef(TKey key) // throws if key not found
    public ref TValue TryGetValueByRef(TKey key, out bool found) 
    public ref TValue TryGetValueByRefOrAdd(TKey key, out bool found, TValue setVal)
}

So also ItemByRef(...) or GetValueByRef(...) are options

Might be good to have something that also works well with a Try prefix?

[Joe4evr]

o.ItemAt(1) = 42;

I'm not sure how I feel about this, it just looks really weird.

Other than that, would be nice to have this with specialized implementations since ElementAt is a LINQ method, so it's frequently a toss up for how it'll perform.

[benaadams]

I'm not sure how I feel about this, it just looks really weird.

o.ItemAt(1) = 42;

Could introduce named indexers in C# as per VB.NET so it could be one of

o.ItemAt[1] = 42;
o.ItemRef[1] = 42;
o.ItemByRef[1] = 42;
o.ByRef[1] = 42;

class MyCollection<T>
{
    public T this[int index] { get; set; }
    public ref T ItemByRef[int index] => throw null;
}

[JonHanna]

Well, I've wanted named indexers since the public beta of C# 1, so my instinct is to favour that, though it opens its own can of worms. My initial thought to the opening post here was that it was another place to wish for named indexers, though.

[mikedn]

I think that having ref in the name is good so I'll vote for ItemRef, RefItem etc. Though I'm not convinced that adding this to general purpose collections like List<T> is a good idea.

[jkotas]

adding this to general purpose collections like List is a good idea

Agree. It is hard to explain and enforce for how long the ref is valid.

[JonHanna]

Is "ref" language-neutral across .NET languages (including those who don't support ref returns, in case they do in the future)?

[benaadams]

VB.NET has ByRef though it can only consume ref returns:

Visual Basic does not allow you to author methods with reference return values, but it does allow you to consume reference return values.

and F# has ref and byref; byref being the ref return consumer

Starting with F# 4.1, you can consume ref returns generated in C#. The result of such a call is a byref<_> pointer.

[VSadov]

Adding the helper to resizable types such as List and Dictionary is indeed controversial since it is possible to cause a resize while holding a reference.

We are not discussing the set of types that could use this though.

So far I hear two popular choices:

[VSadov]

• ItemAt

[VSadov]

• ItemRef

[benaadams]

Like what you did there 😄

[jaredpar]

Adding the helper to resizable types such as List and Dictionary is indeed controversial since it is possible to cause a resize while holding a reference.

This exact discussion came up, at length over and over again, when we did ref returns in Midori. The result is the controversy lasted until we actually checked in the change to expose the ref values from List<T>. After that our code got faster, the debate ended and the hypothetical "using a ref after resize" never actually turned into a real issue.

[benaadams]

This exact discussion came up, at length over and over again, when we did ref returns in Midori. The result is the controversy lasted until we actually checked in the change to expose the ref values from List. After that our code got faster, the debate ended and the hypothetical "using a ref after resize" never actually turned into a real issue.

In "Unite Austin 2017 - Bringing Modern .NET to Unity" they even talk about how to create your own value list with ref returns as one of the exciting new features 😄 https://youtu.be/vJqWn74eCOI?t=30m15s

[JonHanna]

https://github.com/dotnet/corefx/issues/25189#issuecomment-343652212

Could introduce named indexers in C# as per VB.NET so it could be one of

Is this at all likely in anything remotely considered short-term? My number 1 preference would be ItemRef as a named indexer, followed by ItemAt as a named indexer. If we aren't at all likely to see it soon though, I wouldn't want to keep waiting on it.

[jkotas]

how to create your own value list with ref returns

It is absolutely fine to do in custom collections that are optimized for specialized cases like large value types. The mainstream .NET collections are optimized for productivity, not for the last bit of performance in specialized cases.

[benaadams]

The mainstream .NET collections are optimized for productivity,

Currently for a value type you need to do:

for (var i = 0; i < list.Count; i++)
{
    var val = list[i];
    val.X = 0.0;
    list[i] = val;
}

As well as being more perfomant, it is also more productive to mirror an array's behaviour:

for (var i = 0; i < list.Count; i++)
{
    list[i].X = 0.0;
}

[jaredpar]

Getting back to the proposal though. This is meant to provide guidance for the pattern to use for ref accessors on existing types to ensure consistency when we choose to do so. The proposal isn't meant to debate which types we add it to. I'm sure we'll have plenty of fun doing that later 😄

There seem to be two suggestions now for the name: ItemRef and ItemAt.

I'm slightly partial to ItemRef because the name is more descriptive of the action here. I would also be fine with ItemAt.

[benaadams]

ItemRef does seem to have the edge:

[VSadov]

Yes. I think we are nearly done here and ItemRef is going to be the name.

[benaadams]

What would a Try method be e.g. dictionary?

Something like TryGetValueByRef; current being TryGetValue

[terrajobst]

While strictly speaking not an API suggestion, it's a subject we should review as part of the API review process.

[VSadov]

Sure. As a part of the review we need to settle on:
1) General pattern for adding a ref accessor when indexer already present.
Here we propose a ref [readonly] returning method with a name we agree on.
2) Name of the method
Discussion has converged on ItemRef
3) Rough guidance or set of principles on what types should have this.
I would suggest to go through a list of representative types and discuss/record whether we want to add a ref accessor or not and why.

[VSadov]

=== The list of "interesting" types :

• ImmutableArray<T>
• ImmutableList<T>
• ArraySegment<T>
• Stack<T>
• Queue<T>
• List<T>
• ImmutableArray<T>.Builder<T>
• StringBuilder
• ImmutableDictionary<K, V>
• Dictionary<K, V>

== Not very useful / problematic

• String
• Sets, especially sorted sets
• Concurrent collections
• Generally any data structure whose internal organization depends on the value

[benaadams]

String returning a ref readonly char probably isn't useful over just char? Unless there was a corresponding interface that it conformed to.

StringBuilder returning a ref char would have some utility.

[terrajobst]

Video

For data types like Stack<T> and Queue<T> the proposed name ItemRef wouldn't work. Thus, this would be more like a pattern where the name depends on the concept you add a ref-returning method for:

• Indexers -> ItemRef()
• Peek() -> PeekRef()

[karelz]

FYI: The API review discussion was recorded - see https://youtu.be/o5JFZtgaJLs?t=2879 (43 min duration)

[stephentoub]

Moving this back to api-ready-for-review to discuss @jkotas' feedback: https://github.com/dotnet/coreclr/pull/17405#issuecomment-378511865

[OmarTawfik]

@stephentoub not sure why you re-opened this. The concern in the other PR was about other collections (not immutable ones). Did I miss anything?

[stephentoub]

The concern in the other PR was about other collections (not immutable ones). Did I miss anything?

This issue is the one listing out the types to have Item/ValueRef added to it, no?
https://github.com/dotnet/corefx/issues/25189#issuecomment-344688581

[terrajobst]

Video

This issue was about the naming convention (concept suffixing with Ref); it wasn't a particular set of members. It seems there are issues with the particular APIs being added (specifically List<T> and Dictionary<K,V>.)

Please file API review requests for the specific members so we can review those (or alternatively update the issue description to have their signatures and reopen this one).

[stephentoub]

@terrajobst, so what did it mean for this to be tagged as api-approved? It was the concept being approved, not an API? So the APIs in https://github.com/dotnet/coreclr/pull/17405 weren't approved? What about the APIs in https://github.com/dotnet/corefx/pull/25738 that were already merged and cite this issue?

[Neo-Ciber94]

For shorter other possible signature could be just At or Get

public class List<T>
{
        public ref T At(int index);
        //or
        public ref T Get(int index);
}

Also RefAt, GetRef