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.

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

[Proposal]: Adding Index support to existing library types (redux) #8629

Open
1 of 4 tasks
CyrusNajmabadi opened this issue Nov 19, 2024 · 0 comments
Open
1 of 4 tasks
Assignees
Milestone

Comments

@CyrusNajmabadi
Copy link
Member

Adding Index support to existing library types (redux)

Summary

Expand on the ability to implicitly support System.Index and System.Range for additional operations.

For example, the following would be allowed:

void Method(List<T> list)
{
    list.RemoveAt(^1); // no List.RemoveAt(System.Index) method exists.
}

Which would be rewritten to:

void Method(List<T> list)
{
    list.RemoveAt(list.Count - 1);
}

Motivation

Adding index and range support to existing library types made it possible to use System.Index and System.Range in limited cases to support types that didn't know about these new constructs, or which didn't want to update to support them. For example, it was now possible to write either:

void M(List<T> list)
{
    var secondToLastValue = list[^2];
}

// or

Index index;

void M(List<T> list)
{
    var itemAtIndex = list[this.index];    
}

These would translate to the following respectively:

void M(List<T> list)
{
    var secondToLastValue = list[list.Count - 2];
}

// or

Index index;

void M(List<T> list)
{
    var itemAtIndex = list[this.index.GetOffset(list)];    
}

However, while this was good for the general indexing case, it didn't extend any further. So lots of sensible methods (like RemoveAt or InsertAt) still require passing ints and doing the indexing computation manually.

This is solvable at the API level by having new overloads be added either as instance or extension methods. However, this may be a lot of work for an API to do. This can be addressed through source-generators. However, that would involve metadata bloat, as well as indirections through both the extension, and the calls to System.Index.GetOffset. If those alternatives are unpalatable, a pure language approach might then be the desirable way forward.

Detailed design

Given a candidate set of methods, if the methods are not applicable and the methods have a receiver that is Countable and has an this[int] indexer.

Then:

  1. Perform overload resolution again, this time treating any Index parameter as having an implicit conversion to int.
  2. If overload resolution now succeeds then emit the call with the arguments passed along the same as normal exception for any Index arguments that were converted to int. For those arguments, perform the following translation:

Given:

receiver.M(arg1, ..., argN)
  1. When the argument is of the form ^expr2 and the type of expr2 is int, it will be translated to receiver.Length - expr2.
  2. Otherwise, it will be translated as arg.GetOffset(receiver.Length).

Note: The receiver and Length expressions will be spilled as appropriate to ensure any side effects are only executed once.
Note: The oder of evaluation should follow the same rules specified here.

Drawbacks

  1. This could be a bit too broad and might allowing using Indexs where inappropriate. For example:
List<int> list = new();
list.Add(^1);

If this is a concern it could be potentially addressed by requiring that the parameter name be index. We could also consider a requirement that the original type (not the instantiated type) was System.Int32. This would prevent an Index being usable just because a generic happened to be instantiated to int.

It may however just not be a concern in practice and we can allow the most generous approach possible.

  1. Do we want to support something similar for Ranges? For example:
List<T> list = new();
list.RemoveRange([1..^1]);

This does seem nice to support. But it also feels much easier and reasonable for libraries to expose themselves. There are generally orders of magnitude less Range method than Index methods in a cursory exploration of the BCL. Furthermore, it's less clear how such a pattern could necessarily be detected. Unlike the Index approach (which can work by assuming there's an implicit conversion present in some cases), this would not be done with conversions. This would be expansion of one arg to many, similar to how params works. Except that there might be many locations in the argument list this could happen, with very little clarity on how to match them up.

We could also look for specifically int index, int count /*or length*/, but it seems very brittle and difficult to model. Hopefully these are rare enough that just adding overloads in the type itself is a palatable approach.

  1. We could relax the rule about overload resolution needing to fail. We could just add this new implicit conversion and have it always be active. This would be simpler, but could lead to breaking changes as existing code might change meaning. For example, if there was an existing instance method that took an int and an extension that took an Index, then we'd now prefer the former. While this was a break, it might be acceptable under the presumption that the extension existed to supply the functionality the language now provided.

Alternatives

This could be added as instance or extension methods to the types themselves. This could be done manually or using extension methods.

Unresolved questions

Design meetings

https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-09-28.md#ungrouped

@dotnet dotnet locked and limited conversation to collaborators Nov 19, 2024
@CyrusNajmabadi CyrusNajmabadi self-assigned this Nov 19, 2024
@CyrusNajmabadi CyrusNajmabadi added this to the Backlog milestone Nov 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants