Implement design changes for "pattern" Index/Range indexers #34897
Implement design changes for "pattern" Index/Range indexers #34897agocke merged 7 commits intodotnet:masterfrom
Conversation
0c0e1d4 to
2ebfc58
Compare
Implements most of the design changes specified in https://github.com/dotnet/csharplang/blob/c229cae634bd59a6a13b9ed464a4cab782a95e5d/proposals/index-range-changes.md This PR focuses on getting the simple end-to-end scenario working, not focusing entirely on codegen quality. I expect to follow-up later with the optimizations mentioned about eliminating use of the Index/Range helpers entirely if they can be elided.
2ebfc58 to
ae2c987
Compare
|
@333fred Could you take a look at the IOperation stuff and see if it's good enough for a first pass? |
|
Done with review pass (iteration 1). |
|
Done with review pass (iteration 2). Mostly small comments. |
| /// <summary> | ||
| /// The required name for the <c>Count</c> property used in a pattern-based Index or Range indexer. | ||
| /// </summary> | ||
| public const string CountPropertyName = "Count"; |
There was a problem hiding this comment.
public const string CountPropertyName = "Count"; [](start = 8, length = 48)
Why would we want to expose these constants publicly?
There was a problem hiding this comment.
Great question! I don't know, but it seems like all of these are public. Why should these members be different?
There was a problem hiding this comment.
Why should these members be different?
Why should we have these members in this type?
In reply to: 274721942 [](ancestors = 274721942)
There was a problem hiding this comment.
It's where we store the names for other patterns, like foreach or async dispose. I could put them elsewhere, this just seemed like the natural place.
|
@jaredpar @AlekseyTs Any further comments? |
| indexerAccessExpression = BadIndexerExpression(node, expr, analyzedArguments, lookupResult.Error, diagnostics); | ||
| if (TryBindIndexOrRangeIndexer( | ||
| node, | ||
| expr, |
There was a problem hiding this comment.
We commonly style indexer access expressions as receiver[expr], so naming this expr here seems confusing.
There was a problem hiding this comment.
Actually, indexers can have multiple arguments, so it would be receiver[arg1, arg2, arg3] so using expr as the argument name just means we're inserting an expression (which could be any of those components).
* dotnet/master: (495 commits) Roslyn Installer: Stop processes that block VSIX installation. (dotnet#34886) Remove unused helper BeginInvokeOnUIThread Apply a hang mitigating timeout to InvokeOnUIThread Apply a hang mitigating timeout in RestoreNuGetPackages Apply a hang mitigating timeout to WaitForApplicationIdle Fix Value/Ref checks for pattern Index/Range (dotnet#35004) Fix assert in remove unused member analyzer Treat unconstrained type parameters declared in `#nullable disable` context as having an oblivious nullability in case they are substituted with a reference type. (dotnet#34797) Add symbol name to tests. Fix to be the correct name emitted PR feedback Improve IDE0052 diagnostic message for properties with used setter but unused getter Use original definition symbol for fetching control flow graph of generic local functions. Properly treat ambiguous explicit interface implementations (dotnet#34584) Remove the dependence between the order in NullableAnnotation and metadata attribute values (dotnet#34909) Fix warning level test. Fix bootstrap on Linux/Mac (dotnet#34978) disable completion for immediate window commands (dotnet#32631) Updated PreviewWarning color Implement design changes for "pattern" Index/Range indexers (dotnet#34897) Add IVTs to 16.1 version of RemoteLS ...
Implements most of the design changes specified in
https://github.com/dotnet/csharplang/blob/c229cae634bd59a6a13b9ed464a4cab782a95e5d/proposals/index-range-changes.md
This PR focuses on getting the simple end-to-end scenario working,
not focusing entirely on codegen quality. I expect to follow-up later
with the optimizations mentioned about eliminating use of the
Index/Range helpers entirely if they can be elided.