Compare Nullable modifiers while comparing type symbols by default. #30770
Conversation
AlekseyTs
added
Area-Compilers
New Language Feature - Nullable Reference Types
|
Test windows_release_vs-integration_prtest please |
| @@ -2905,7 +2905,7 @@ private static void GetAllCandidates(Dictionary<TypeSymbolWithAnnotations, TypeS | |||
| { | |||
| candidates.Remove(candidate); | |||
| } | |||
| else if (bound.Equals(candidate, TypeCompareKind.IgnoreDynamicAndTupleNames)) | |||
| else if (bound.Equals(candidate, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes)) | |||
There was a problem hiding this comment.
This seems to be used relatively often. Might be worth adding an explicit option in the enum. #Pending
| @@ -512,7 +512,7 @@ public int GetHashCode(Symbol member) | |||
| } | |||
|
|
|||
| if (_considerReturnType && member.GetMemberArity() == 0 && | |||
| (_typeComparison & TypeCompareKind.IgnoreCustomModifiersAndArraySizesAndLowerBounds) == 0) // If it is generic, then type argument might be in return type. | |||
| (_typeComparison & TypeCompareKind.AllIgnoreOptions) == 0) // If it is generic, then type argument might be in return type. | |||
There was a problem hiding this comment.
Isn't this ignoring new things besides nullability? Why the change? #Resolved
There was a problem hiding this comment.
Isn't this ignoring new things besides nullability? Why the change?
At this point I believe the change is necessary to ensure correctness because at the moment some implementations of GetHashCode on symbols actually sensitive to things that we are supposed to ignore. See #30673. I'll add a note to that issue that once we ensure that all GetHashCode implementations do the right thing, this condition probably can be removed.
In reply to: 228374243 [](ancestors = 228374243)
|
@cston Please review |
|
|
||
| [Fact] | ||
| [WorkItem(30677, "https://github.com/dotnet/roslyn/issues/30677")] | ||
| public void TestErrorsImplementingGenericNestedInterfaces_Explicit_IncorrectPartialQualification() |
There was a problem hiding this comment.
Incorrect [](start = 75, length = 9)
What does Incorrect refer to? #Resolved
There was a problem hiding this comment.
What does Incorrect refer to?
This unit-test is a clone (possibly partial) of the other unit-test, the name is inherited from the original unit-test.
In reply to: 228689077 [](ancestors = 228689077)
|
Consider targeting master rather than features/NullableReferenceTypes. |
I think it is used. In reply to: 433565246 [](ancestors = 433565246) Refers to: src/Compilers/Core/Portable/Symbols/TypeCompareKind.cs:13 in d74070e. [](commit_id = d74070e, deletion_comment = False) |
Sorry, I was referring to the comment. I think the comment can be removed. In reply to: 433985024 [](ancestors = 433985024,433565246) Refers to: src/Compilers/Core/Portable/Symbols/TypeCompareKind.cs:13 in d74070e. [](commit_id = d74070e, deletion_comment = False) |
Sure. Thanks for the clarification. Will remove In reply to: 433987578 [](ancestors = 433987578,433985024,433565246) Refers to: src/Compilers/Core/Portable/Symbols/TypeCompareKind.cs:13 in d74070e. [](commit_id = d74070e, deletion_comment = False) |
No description provided.