Add INamedType.Construct overload with nullable annotations#37271
Add INamedType.Construct overload with nullable annotations#37271cston merged 3 commits intodotnet:masterfrom
Conversation
|
@dotnet/roslyn-compiler, @jasonmalinowski, please review. |
| return Construct(builder.ToImmutableAndFree(), unbound: false); | ||
| } | ||
|
|
||
| INamedTypeSymbol INamedTypeSymbol.Construct(ImmutableArray<ITypeSymbol> typeArguments, ImmutableArray<CodeAnalysis.NullableAnnotation> typeArgumentNullableAnnotations) |
There was a problem hiding this comment.
Is there any reason to allow a default typeArgumentNullableAnnotations? Feels like the user is calling the wrong overload if they pass default instead of calling the other version. #ByDesign
There was a problem hiding this comment.
Ideally callers should be able to use a single overload.
In reply to: 304053732 [](ancestors = 304053732)
|
|
||
| [Fact] | ||
| [WorkItem(37161, "https://github.com/dotnet/roslyn/issues/37161")] | ||
| public void EmitPrivateMetadata_ExplicitImplementation() |
There was a problem hiding this comment.
Is this add just a weird merge or something? Doesn't seem relevant. #ByDesign
There was a problem hiding this comment.
|
Done review pass (commit 3) |
|
|
||
| public INamedTypeSymbol Construct(ImmutableArray<ITypeSymbol> typeArguments, ImmutableArray<NullableAnnotation> typeArgumentNullableAnnotations) | ||
| { | ||
| return WrappedSymbol.Construct(typeArguments, typeArgumentNullableAnnotations); |
There was a problem hiding this comment.
; [](start = 94, length = 1)
.WithNullability(Nullability)
There was a problem hiding this comment.
Correct: should work like the method above.
gafter
left a comment
There was a problem hiding this comment.
Other than the small bug in NamedTypeSymbolWithNullableAnnotation, looks good to me.
| Return Construct(builder.ToImmutableAndFree()) | ||
| End Function | ||
|
|
||
| Private Function INamedTypeSymbol_Construct(typeArguments As ImmutableArray(Of ITypeSymbol), typeArgumentNullableAnnotations As ImmutableArray(Of CodeAnalysis.NullableAnnotation)) As INamedTypeSymbol Implements INamedTypeSymbol.Construct |
There was a problem hiding this comment.
I think we should throw NotImplemented for VB
jcouv
left a comment
There was a problem hiding this comment.
LGTM Thanks (iteration 3) modulo VB observation (should throw NotImplemented, imo)
Fixes #36046