Add and implement IImplicitIndexerReferenceOperation
#58200
Conversation
| <Node Name="IImplicitIndexerReferenceOperation" Base="IOperation" ChildrenOrder="Instance,Argument" HasType="true"> | ||
| <Comments> | ||
| <summary> | ||
| Represents a reference to an implicit System.Index or System.Range indexer over non-array type. |
| <Property Name="LengthSymbol" Type="ISymbol"> | ||
| <Comments> | ||
| <summary> | ||
| The <c>Length</c> or <c>Count</c> property that can be used to fetch the length value. |
There was a problem hiding this comment.
Perhaps will be used?
I intentionally used "can" because we might never use it. It all depends.
There was a problem hiding this comment.
In that case, I think might sounds better. But it's ok to leave as is if you prefer.
| <Property Name="IndexerSymbol" Type="ISymbol"> | ||
| <Comments> | ||
| <summary> | ||
| Symbol for the underlying indexer or a slice method that is being used to implement the implicit indexer. |
| IOperation argument = VisitRequired(operation.Argument); | ||
| IOperation instance = PopOperand(); | ||
| PopStackFrame(frame); | ||
| return new ImplicitIndexerReferenceOperation(instance, argument, operation.LengthSymbol, operation.IndexerSymbol, semanticModel: null, |
There was a problem hiding this comment.
Should we be lowering the indexer to its final form here? I would have somewhat expected it to be turned into the series of method/property calls that make up an indexer. #Resolved
There was a problem hiding this comment.
Should we be lowering the indexer to its final form here? I would have somewhat expected it to be turned into the series of method/property calls that make up an indexer.
There is no requirement to lower things in CFG. We can accurately reflect the control flow by using the original node.
333fred
left a comment
333fred
left a comment
There was a problem hiding this comment.
LGTM (commit 2) with minor doc adjustments.
|
@dotnet/roslyn-compiler For the second review. |
|
@dotnet/roslyn-compiler For the second review. |
1 similar comment
|
@dotnet/roslyn-compiler For the second review. |
jcouv
left a comment
jcouv
left a comment
There was a problem hiding this comment.
Compiler side LGTM (iteration 4). Not sure what the IDE portion has to do with this PR, consider splitting.
|
|
||
| [CompilerTrait(CompilerFeature.IOperation, CompilerFeature.Dataflow)] | ||
| [Fact] | ||
| public void ImplicitIndexIndexer_ControlFlowInInstanceAndArgument_String() |
There was a problem hiding this comment.
Thanks much for the descriptive test names. It's much easier to follow the purpose than numbers
Unfortunately CI is failing without these changes. Apparently analyzers are able to make sense of more code after the IOperation change and are able to detect more violations. |
ghost
added this to the
4 participants
Closes #58079.