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.

Sign up for GitHub

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

CreateTupleTypeSymbol shouldn’t reset nullability element types to None if elementNullableAnnotations is a default array. #40348

Merged
merged 1 commit into from
Dec 13, 2019
Merged

CreateTupleTypeSymbol shouldn’t reset nullability element types to None if elementNullableAnnotations is a default array. #40348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3131,8 +3131,9 @@ protected override INamedTypeSymbol CommonCreateTupleTypeSymbol(
var typesBuilder = ArrayBuilder<TypeWithAnnotations>.GetInstance(elementTypes.Length);
for (int i = 0; i < elementTypes.Length; i++)
{
var elementType = elementTypes[i].EnsureCSharpSymbolOrNull($"{nameof(elementTypes)}[{i}]");
var annotation = elementNullableAnnotations.IsDefault ? NullableAnnotation.Oblivious : elementNullableAnnotations[i].ToInternalAnnotation();
ITypeSymbol typeSymbol = elementTypes[i];
var elementType = typeSymbol.EnsureCSharpSymbolOrNull($"{nameof(elementTypes)}[{i}]");
var annotation = (elementNullableAnnotations.IsDefault ? typeSymbol.NullableAnnotation : elementNullableAnnotations[i]).ToInternalAnnotation();
typesBuilder.Add(TypeWithAnnotations.Create(elementType, annotation));
}

Expand Down
122 changes: 102 additions & 20 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTupleTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6295,7 +6295,7 @@ class C

[Fact]
[WorkItem(36047, "https://github.com/dotnet/roslyn/issues/36047")]
public void CreateTupleTypeSymbol_UnderlyingType_DefaultArgs()
public void CreateTupleTypeSymbol_UnderlyingType_DefaultArgs_01()
{
var source =
@"class Program
Expand All @@ -6307,33 +6307,74 @@ public void CreateTupleTypeSymbol_UnderlyingType_DefaultArgs()
var underlyingType = tuple1.TupleUnderlyingType;

var tuple2 = comp.CreateTupleTypeSymbol(underlyingType);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, default, default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, default, default, default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, default(ImmutableArray<string>), default(ImmutableArray<Location>), default(ImmutableArray<CodeAnalysis.NullableAnnotation>));
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, elementNames: default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, elementLocations: default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, elementNullableAnnotations: default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));
}

[Fact]
[WorkItem(36047, "https://github.com/dotnet/roslyn/issues/36047")]
public void CreateTupleTypeSymbol_ElementTypes_DefaultArgs()
[WorkItem(40105, "https://github.com/dotnet/roslyn/issues/40105")]
public void CreateTupleTypeSymbol_UnderlyingType_DefaultArgs_02()
{
var source =
@"class Program
{
(string,
#nullable enable
string, string?) F;
}";
var comp = (Compilation)CreateCompilation(source);
var tuple1 = (INamedTypeSymbol)((IFieldSymbol)comp.GetMember("Program.F")).Type;
var underlyingType = tuple1.TupleUnderlyingType;

var tuple2 = comp.CreateTupleTypeSymbol(underlyingType);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, default, default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, default, default, default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, default(ImmutableArray<string>), default(ImmutableArray<Location>), default(ImmutableArray<CodeAnalysis.NullableAnnotation>));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, elementNames: default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, elementLocations: default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(underlyingType, elementNullableAnnotations: default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));
}

[Fact]
[WorkItem(36047, "https://github.com/dotnet/roslyn/issues/36047")]
public void CreateTupleTypeSymbol_ElementTypes_DefaultArgs_01()
{
var source =
@"class Program
Expand All @@ -6345,28 +6386,69 @@ public void CreateTupleTypeSymbol_ElementTypes_DefaultArgs()
var elementTypes = tuple1.TupleElements.SelectAsArray(e => e.Type);

var tuple2 = comp.CreateTupleTypeSymbol(elementTypes);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, default, default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, default, default, default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, default(ImmutableArray<string>), default(ImmutableArray<Location>), default(ImmutableArray<CodeAnalysis.NullableAnnotation>));
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, elementNames: default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, elementLocations: default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, elementNullableAnnotations: default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));
}

[Fact]
[WorkItem(36047, "https://github.com/dotnet/roslyn/issues/36047")]
[WorkItem(40105, "https://github.com/dotnet/roslyn/issues/40105")]
public void CreateTupleTypeSymbol_ElementTypes_DefaultArgs_02()
{
var source =
@"class Program
{
(string,
#nullable enable
string, string?) F;
}";
var comp = (Compilation)CreateCompilation(source);
var tuple1 = (INamedTypeSymbol)((IFieldSymbol)comp.GetMember("Program.F")).Type;
var elementTypes = tuple1.TupleElements.SelectAsArray(e => e.Type);

var tuple2 = comp.CreateTupleTypeSymbol(elementTypes);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, default, default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, default, default, default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, default(ImmutableArray<string>), default(ImmutableArray<Location>), default(ImmutableArray<CodeAnalysis.NullableAnnotation>));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, elementNames: default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, elementLocations: default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

tuple2 = comp.CreateTupleTypeSymbol(elementTypes, elementNullableAnnotations: default);
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));
}
Copy link
Member

@cston cston Dec 13, 2019 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} [](start = 8, length = 1)

Are we testing cases where elementTypes has annotations and so does elementNullableAnnotations? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we testing cases where elementTypes has annotations and so does elementNullableAnnotations?

I believe existing tests below sufficiently cover these cases.

In reply to: 357434927 [](ancestors = 357434927)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any test where the annotations on elementTypes are not None and distinct from elementNullableAnnotations.

In reply to: 357436843 [](ancestors = 357436843,357434927)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any test where the annotations on elementTypes are not None and distinct from elementNullableAnnotations.

This PR is focused on scenarios where elementNullableAnnotations is default.

In reply to: 357776096 [](ancestors = 357776096,357436843,357434927)


[Fact]
Expand All @@ -6383,15 +6465,15 @@ public void CreateTupleTypeSymbol_UnderlyingType_WithNullableAnnotations_01()
var underlyingType = tuple1.TupleUnderlyingType;

var tuple2 = comp.CreateTupleTypeSymbol(underlyingType, elementNullableAnnotations: default);
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));

var e = Assert.Throws<ArgumentException>(() => comp.CreateTupleTypeSymbol(underlyingType, elementNullableAnnotations: ImmutableArray<CodeAnalysis.NullableAnnotation>.Empty));
Assert.Contains(CodeAnalysisResources.TupleElementNullableAnnotationCountMismatch, e.Message);

tuple2 = comp.CreateTupleTypeSymbol(
underlyingType,
elementNullableAnnotations: ImmutableArray.Create(CodeAnalysis.NullableAnnotation.None, CodeAnalysis.NullableAnnotation.None));
Assert.True(tuple1.Equals(tuple2));
Assert.True(tuple1.Equals(tuple2, TypeCompareKind.ConsiderEverything));
Assert.Equal("(System.Int32, System.String)", tuple2.ToTestDisplayString(includeNonNullable: true));

tuple2 = comp.CreateTupleTypeSymbol(
Expand Down