Add IOperation support for variations of object creation expressions … #20962
Conversation
…(type parameter, nopia and dynamic) Fixes dotnet#20119 and dotnet#20491
| /// <summary> | ||
| /// List of applicable symbols that are dynamically bound to the <see cref="Name"/>. | ||
| /// </summary> | ||
| ImmutableArray<ISymbol> ApplicableSymbols { get; } |
There was a problem hiding this comment.
Could this be ImmutableArray<IMethodSymbol>?
There was a problem hiding this comment.
This interface will also be implemented by BoundDynamicIndexerAccess, which has applicable property symbols.
| ImmutableArray<ISymbol> applicableSymbols = StaticCast<ISymbol>.From(boundDynamicObjectCreationExpression.ApplicableMethods); | ||
| Lazy<ImmutableArray<IOperation>> arguments = new Lazy<ImmutableArray<IOperation>>(() => boundDynamicObjectCreationExpression.Arguments.SelectAsArray(n => Create(n))); | ||
| ImmutableArray<string> argumentNames = boundDynamicObjectCreationExpression.ArgumentNamesOpt.NullToEmpty(); | ||
| ImmutableArray<RefKind> argumentRefKinds = boundDynamicObjectCreationExpression.ArgumentRefKindsOpt.NullToEmpty(); |
There was a problem hiding this comment.
ArgumentNames and ArgumentRefKinds should be default rather than Empty if not set.
There was a problem hiding this comment.
I thought we need to expose non default ImmutableArray for all public APIs. @dotnet/analyzer-ioperation, what is the preference here?
There was a problem hiding this comment.
We could use Empty here if we're using Empty in all other cases. In the bound tree though, these properties are either unset (default) or populated with the same number of entries as Arguments.
There was a problem hiding this comment.
Filed #20974 to discuss this at the design meeting and make the behavior consistent across the factory.
| /// <remarks> | ||
| /// This interface is reserved for implementation by its associated APIs. We reserve the right to | ||
| /// change it in the future. | ||
| /// </remarks> |
There was a problem hiding this comment.
Looks like this comment was copied from the interface unnecessarily. Same for other classes here.
There was a problem hiding this comment.
I will fix the comment.
| @@ -241,7 +240,7 @@ private void VisitArray<T>(IEnumerable<T> list, string header, bool logElementCo | |||
| LogString($"{header}{elementCount}:"); | |||
| LogNewLine(); | |||
| Indent(); | |||
| VisitArray(list); | |||
| elementVisitor(list); | |||
There was a problem hiding this comment.
Perhaps elementVisitor could be Action<T> with the foreach handled here.
There was a problem hiding this comment.
Good idea, will refactor.
|
LGTM |
| } | ||
| } | ||
|
|
||
| internal void VisitArrayElement<T>(T operation) where T : IOperation |
There was a problem hiding this comment.
Is T necessary or could this be VisitArrayElement(IOperation operation)?
There was a problem hiding this comment.
It is necessary as it is also passed as a delegate to VisitArrayCommon in the OperationTreeVerifier.
|
windows_release_vs-integration_prtest failed due to flaky test failure: #15497 |
|
@dotnet-bot retest windows_release_vs-integration_prtest please |
| @@ -1904,6 +1904,7 @@ class C2 : C1 | |||
| Variables: Local_1: C1 c1 | |||
| Initializer: IConversionExpression (ConversionKind.Cast, Implicit) (OperationKind.ConversionExpression, Type: C1) (Syntax: 'new T()') | |||
| ITypeParameterObjectCreationExpression (OperationKind.TypeParameterObjectCreationExpression, Type: T) (Syntax: 'new T()') | |||
| null | |||
There was a problem hiding this comment.
I believe this is what Jinu's PR is fixing.
…(type parameter and dynamic)
Fixes #20119 and #20491